Do not reset navigation state when BeforeUnload is cancelled by a commit

content/test/content_browser_test_utils_internal.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_TEST_CONTENT_BROWSER_TEST_UTILS_INTERNAL_H_
 #define CONTENT_TEST_CONTENT_BROWSER_TEST_UTILS_INTERNAL_H_
 
 // A collection of functions designed for use with content_shell based browser
 // tests internal to the content/ module.
 // Note: If a function here also works with browser_tests, it should be in
 // the content public API.
 
 #include <string>
 #include <vector>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "cc/surfaces/surface_id.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "url/gurl.h"
 
 namespace cc {
 class SurfaceManager;
 }
 
 namespace content {
 
 class FrameTreeNode;
+class MessageLoopRunner;
 class RenderWidgetHostViewChildFrame;
 class Shell;
 class SiteInstance;
 class ToRenderFrameHost;
 
 // Navigates the frame represented by |node| to |url|, blocking until the
 // navigation finishes.
 void NavigateFrameToURL(FrameTreeNode* node, const GURL& url);
 
+// Sets the DialogManager to proceed by default or not when showing a
+// BeforeUnload dialog.
+void SetShouldProceedOnBeforeUnload(Shell* shell, bool proceed);
+
 // Creates compact textual representations of the state of the frame tree that
 // is appropriate for use in assertions.
 //
 // The diagrams show frame tree structure, the SiteInstance of current frames,
 // presence of pending frames, and the SiteInstances of any and all proxies.
 // They look like this:
 //
 //        Site A (D pending) -- proxies for B C
 //          |--Site B --------- proxies for A C
 //          +--Site C --------- proxies for B A
@@ skipping 49 matching lines @@
   void RequestBeginning(
       net::URLRequest* request,
       content::ResourceContext* resource_context,
       content::AppCacheService* appcache_service,
       ResourceType resource_type,
       ScopedVector<content::ResourceThrottle>* throttles) override;
 
   GURL url_;
 };
 
+// This class can be used to pause and resume navigations, based on a URL
+// match. Note that it only keeps track of one navigation at a time.
+class TestNavigationManager : public WebContentsObserver {
+ public:
+  // Currently this monitors any frame in WebContents.
+  // TODO(clamy): Extend this class so that it can monitor a specific frame.
+  TestNavigationManager(WebContents* web_contents, const GURL& url);
+  ~TestNavigationManager() override;
+
+  // Waits until the navigation request is ready to be sent to the network
+  // stack. The navigation will be paused until it is resumed by calling
+  // ResumeNavigation.
+  void WaitForWillStartRequest();
+
+  // Waits until the navigation has been finished. Users of this method should
+  // first use WaitForWillStartRequest, then call ResumeNavigation, and only

[nasko, 2016/03/30 15:47:52]

Why this limitation? Simple to implement now or intended API behavior for test
writers?

[carlosk, 2016/03/31 09:48:58]

I'm guessing the idea is to expand this helper class to cover more intermediary
navigation steps. If that's the intention the I have an 
API suggestion.

When TNM is created it pauses at the earliest step possible. Then when a
Wait-call is made it allows navigations to cross all previous pause moments
until the specific one for the call is reached. Future Wait-calls work the same
way. ResumeNavigation would not be needed so much.

Good to have: If that moment is not reachable anymore in for the current
navigation then fail the test.

Now looking at your TODO below it might be what you were already planning to
do...

[clamy, 2016/04/01 10:19:00]

Yes. Just it was easier to write the first version that way.

+  // then WaitForNavigationFinished.
+  // TODO(clamy): Do not pause the navigation in WillStartRequest by default.
+  void WaitForNavigationFinished();
+
+  // Resumes the navigation if it was previously paused.
+  void ResumeNavigation();

[Charlie Reis, 2016/03/30 19:04:58]

nit: Let's list this between WaitForWillStartRequest and
WaitForNavigationFinished, so that they're in the order they're expected to be
called.

[clamy, 2016/04/01 10:19:00]

Done.

+
+ private:
+  // WebContentsObserver implementation.
+  void DidStartNavigation(NavigationHandle* handle) override;
+  void DidFinishNavigation(NavigationHandle* handle) override;
+
+  // Called when the NavigationThrottle pauses the navigation in
+  // WillStartRequest.
+  void OnWillStartRequest();
+
+  const GURL url_;
+  bool navigation_paused_;
+  NavigationHandle* handle_;
+  scoped_refptr<MessageLoopRunner> loop_runner_;
+
+  base::WeakPtrFactory<TestNavigationManager> weak_factory_;
+};
+
 // Helper class to assist with hit testing surfaces in multiple processes.
 // WaitForSurfaceReady() will only return after a Surface from |target_view|
 // has been composited in the top-level frame's Surface. At that point,
 // browser process hit testing to target_view's Surface can succeed.
 class SurfaceHitTestReadyNotifier {
  public:
   SurfaceHitTestReadyNotifier(RenderWidgetHostViewChildFrame* target_view);
   ~SurfaceHitTestReadyNotifier() {}
 
   void WaitForSurfaceReady();
 
  private:
   bool ContainsSurfaceId(cc::SurfaceId container_surface_id);
 
   cc::SurfaceManager* surface_manager_;
   cc::SurfaceId root_surface_id_;
   RenderWidgetHostViewChildFrame* target_view_;
 
   DISALLOW_COPY_AND_ASSIGN(SurfaceHitTestReadyNotifier);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_TEST_CONTENT_BROWSER_TEST_UTILS_INTERNAL_H_