PlzNavigate: properly execute BeforeUnload on renderer initiated navigations

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

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008 Alp Toker <alp@atoker.com>
  * Copyright (C) Research In Motion Limited 2009. All rights reserved.
  * Copyright (C) 2011 Kris Jordan <krisjordan@gmail.com>
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ skipping 1301 matching lines @@
         for (i = 0; i < targetFrames.size(); i++) {
             if (!targetFrames[i]->tree().isDescendantOf(m_frame))
                 continue;
             if (!targetFrames[i]->document()->dispatchBeforeUnloadEvent(page->chromeClient(), isReload, didAllowNavigation))
                 break;
         }
 
         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;
 }
 
 bool FrameLoader::shouldContinueForNavigationPolicy(const ResourceRequest& request, const SubstituteData& substituteData,
     DocumentLoader* loader, ContentSecurityPolicyDisposition shouldCheckMainWorldContentSecurityPolicy,
     NavigationType type, NavigationPolicy policy, bool replacesCurrentHistoryItem, bool isClientRedirect)
 {
     // Don't ask if we are loading an empty URL.
     if (request.url().isEmpty() || substituteData.isValid())
         return true;
@@ skipping 35 matching lines @@
     ASSERT(client()->hasWebView());
     if (m_frame->document()->pageDismissalEventBeingDispatched() != Document::NoDismissal)
         return;
 
     NavigationType navigationType = determineNavigationType(type, frameLoadRequest.resourceRequest().httpBody() || frameLoadRequest.form(), frameLoadRequest.triggeringEvent());
     frameLoadRequest.resourceRequest().setRequestContext(determineRequestContextFromNavigationType(navigationType));
     frameLoadRequest.resourceRequest().setFrameType(m_frame->isMainFrame() ? WebURLRequest::FrameTypeTopLevel : WebURLRequest::FrameTypeNested);
     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();
     detachDocumentLoader(m_provisionalDocumentLoader);
 
     // beforeunload fired above, and detaching a DocumentLoader can fire
     // events, which can detach this frame.
     if (!m_frame->host())
         return;
 
@@ skipping 176 matching lines @@
     // FIXME: We need a way to propagate insecure requests policy flags to
     // out-of-process frames. For now, we'll always use default behavior.
     if (!parentFrame->isLocalFrame())
         return nullptr;
 
     ASSERT(toLocalFrame(parentFrame)->document());
     return toLocalFrame(parentFrame)->document()->insecureNavigationsToUpgrade();
 }
 
 } // namespace blink