PlzNavigate: properly execute BeforeUnload on renderer initiated navigations

third_party/WebKit/Source/core/loader/FrameLoader.cpp

Index: third_party/WebKit/Source/core/loader/FrameLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index b165a2e788363496aed321df4557f0904602bf14..271a2efd86526d206d93a67b030c3b311417dcc6 100644
--- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -1319,6 +1319,10 @@ bool FrameLoader::shouldClose(bool isReload)
         if (i == targetFrames.size())
             shouldClose = true;
     }
+
+    if (!m_frame->host())

[dcheng, 2016/04/15 17:21:42]

How do we get into a case where we call this on a detached frame?

[clamy, 2016/04/25 11:36:55]

There's a specific layout test checking for what happens when the BeforeUnload
event detaches the frame.

[dcheng, 2016/04/26 06:32:31]

Right, but why didn't we need to previously check for this?

[clamy, 2016/04/26 10:07:09]

I think this i sbecause this function was only called for cross-process
navigations, and only for the main frame. I don't think it's possible for the
main frame to detach itself. However, the patch has it called on all
renderer-initiated navigations, including subframes when PlzNavigate is enabled.
And subframes can detach themselves.

+        return false;
+
     return shouldClose;
 }
 
@@ -1374,7 +1378,13 @@ void FrameLoader::startLoad(FrameLoadRequest& frameLoadRequest, FrameLoadType ty
     ResourceRequest& request = frameLoadRequest.resourceRequest();
     if (!shouldContinueForNavigationPolicy(request, frameLoadRequest.substituteData(), nullptr, frameLoadRequest.shouldCheckMainWorldContentSecurityPolicy(), navigationType, navigationPolicy, type == FrameLoadTypeReplaceCurrentItem, frameLoadRequest.clientRedirect() == ClientRedirectPolicy::ClientRedirect))
         return;
-    if (!shouldClose(navigationType == NavigationTypeReload))
+
+    // Execute the BeforeUnload event, which may cancel the navigation.
+    // PlzNavigate: bypassBeforeUnload can be true when committing a navigation
+    // that has already made a network request. Do not execute the BeforeUnload
+    // event in that case, since it has already executed just before the
+    // navigation was sent to the network.
+    if (!request.bypassBeforeUnload() && !shouldClose(navigationType == NavigationTypeReload))
         return;
 
     m_frame->document()->cancelParsing();