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

content/test/content_browser_test_utils_internal.h

Index: content/test/content_browser_test_utils_internal.h
diff --git a/content/test/content_browser_test_utils_internal.h b/content/test/content_browser_test_utils_internal.h
index f4a4ddea552c7b37746fc87275a56e3bf0beaad2..94989f1a2d70af27017b4d81e43fa40cb0f6f92d 100644
--- a/content/test/content_browser_test_utils_internal.h
+++ b/content/test/content_browser_test_utils_internal.h
@@ -14,8 +14,11 @@
 #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 {
@@ -25,6 +28,7 @@ class SurfaceManager;
 namespace content {
 
 class FrameTreeNode;
+class MessageLoopRunner;
 class RenderWidgetHostViewChildFrame;
 class Shell;
 class SiteInstance;
@@ -34,6 +38,10 @@ class ToRenderFrameHost;
 // 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.
 //
@@ -103,6 +111,46 @@ class NavigationStallDelegate : public ResourceDispatcherHostDelegate {
   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,