Keep track in the browser of which frames have onunload and onbeforeunload handlers.

content/browser/frame_host/navigator_impl_unittest.cc

Index: content/browser/frame_host/navigator_impl_unittest.cc
diff --git a/content/browser/frame_host/navigator_impl_unittest.cc b/content/browser/frame_host/navigator_impl_unittest.cc
index 0d2f9b0f29c1790237b866dece5c8c94d5dc91f7..dd68bf558ed86384c43a5b4c7fa0a1865c96a8a5 100644
--- a/content/browser/frame_host/navigator_impl_unittest.cc
+++ b/content/browser/frame_host/navigator_impl_unittest.cc
@@ -269,6 +269,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   const GURL kUrl2("http://www.chromium.org/");
 
   contents()->NavigateAndCommit(kUrl1);
+  main_test_rfh()->set_has_beforeunload_handlers();
 
   // Start a new navigation.
   FrameTreeNode* node = main_test_rfh()->frame_tree_node();
@@ -307,14 +308,8 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, BeginNavigation) {
   RequestNavigation(subframe_node, kUrl2);
   NavigationRequest* subframe_request = subframe_node->navigation_request();
 
-  // We should be waiting for the BeforeUnload event to execute in the subframe.
   ASSERT_TRUE(subframe_request);
-  EXPECT_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE,
-            subframe_request->state());
-  EXPECT_TRUE(subframe_rfh->is_waiting_for_beforeunload_ack());
 
-  // Simulate the BeforeUnload ACK. The navigation should start.
-  subframe_rfh->SendBeforeUnloadACK(true);

[nasko, 2017/03/29 23:31:26]

Aren't we losing coverage for a beforeunload handler present, which allows
navigation to proceed? 

[jam, 2017/03/30 14:53:22]

we already have test cases for this, i.e. all the browser tests that do have
beforeunload handlers. IMO all these unit tests aren't really testing this,
since it's all mocked up and using different code paths than production. they
just had to do this to not hang. since the unit tests should be about testing
one thing, and none of these tests are about onbeforeunload, IMO this is an
improvement :)

   TestNavigationURLLoader* subframe_loader =
       GetLoaderForNavigationRequest(subframe_request);
   ASSERT_TRUE(subframe_loader);
@@ -341,15 +336,11 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, BeginNavigation) {
   RequestNavigation(root_node, kUrl3);
   NavigationRequest* main_request = root_node->navigation_request();
   ASSERT_TRUE(main_request);
-  EXPECT_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE,
-            main_request->state());
 
   // Main frame navigation to a different site should use a speculative
   // RenderFrameHost.
   EXPECT_TRUE(GetSpeculativeRenderFrameHost(root_node));
 
-  // Simulate a BeforeUnloadACK IPC on the main frame.
-  main_test_rfh()->SendBeforeUnloadACK(true);
   TestNavigationURLLoader* main_loader =
       GetLoaderForNavigationRequest(main_request);
   EXPECT_EQ(kUrl3, main_request->common_params().url);