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

content/browser/frame_host/navigator_impl_unittest.cc

 // 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.
 
 #include <stdint.h>
 
 #include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
@@ skipping 251 matching lines @@
   EXPECT_EQ(site_instance_id_1, main_test_rfh()->GetSiteInstance()->GetId());
 }
 
 // PlzNavigate: Test that a beforeUnload denial cancels the navigation.
 TEST_F(NavigatorTestWithBrowserSideNavigation,
        BeforeUnloadDenialCancelNavigation) {
   const GURL kUrl1("http://www.google.com/");
   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();
   RequestNavigation(node, kUrl2);
   NavigationRequest* request = node->navigation_request();
   ASSERT_TRUE(request);
   EXPECT_TRUE(request->browser_initiated());
   EXPECT_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE, request->state());
   EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
   RenderFrameDeletedObserver rfh_deleted_observer(
@@ skipping 18 matching lines @@
   // Add a subframe.
   FrameTreeNode* root_node = contents()->GetFrameTree()->root();
   TestRenderFrameHost* subframe_rfh = main_test_rfh()->AppendChild("Child");
   ASSERT_TRUE(subframe_rfh);
 
   // Start a navigation at the subframe.
   FrameTreeNode* subframe_node = subframe_rfh->frame_tree_node();
   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);
   EXPECT_EQ(NavigationRequest::STARTED, subframe_request->state());
   EXPECT_EQ(kUrl2, subframe_request->common_params().url);
   EXPECT_EQ(kUrl2, subframe_loader->request_info()->common_params.url);
   // First party for cookies url should be that of the main frame.
   EXPECT_EQ(kUrl1, subframe_loader->request_info()->first_party_for_cookies);
   EXPECT_FALSE(subframe_loader->request_info()->is_main_frame);
   EXPECT_TRUE(subframe_loader->request_info()->parent_is_main_frame);
   EXPECT_TRUE(subframe_request->browser_initiated());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(root_node));
 
   // Subframe navigations should never create a speculative RenderFrameHost,
   // unless site-per-process is enabled. In that case, as the subframe
   // navigation is to a different site and is still ongoing, it should have one.
   if (AreAllSitesIsolatedForTesting()) {
     EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node));
   } else {
     EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node));
   }
 
   // Now start a navigation at the root node.
   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);
   EXPECT_EQ(kUrl3, main_loader->request_info()->common_params.url);
   EXPECT_EQ(kUrl3, main_loader->request_info()->first_party_for_cookies);
   EXPECT_TRUE(main_loader->request_info()->is_main_frame);
   EXPECT_FALSE(main_loader->request_info()->parent_is_main_frame);
   EXPECT_TRUE(main_request->browser_initiated());
   // BeforeUnloadACK was received from the renderer so the navigation should
   // have started.
@@ skipping 917 matching lines @@
 
   // Navigate to the new URL.
   contents()->NavigateAndCommit(kUrl2);
 
   // Check the feature policy after navigation.
   FeaturePolicy* final_feature_policy = main_test_rfh()->get_feature_policy();
   ASSERT_EQ(original_feature_policy, final_feature_policy);
 }
 
 }  // namespace content