PlzNavigate: Fix the http/tests/security/cross-frame-access-parent-isolated-world.html and http/tes…

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.
@@ skipping 1337 matching lines @@
         WebEventListenerClass::TouchStartOrMove,
         WebEventListenerProperties::Nothing);
     m_frame->page()->chromeClient().setEventListenerProperties(
         WebEventListenerClass::MouseWheel, WebEventListenerProperties::Nothing);
     m_frame->page()->chromeClient().setEventListenerProperties(
         WebEventListenerClass::TouchEndOrCancel,
         WebEventListenerProperties::Nothing);
   }
 
   client()->transitionToCommittedForNewPage();
-  m_frame->navigationScheduler().cancel();
+
+  // PlzNavigate: We need to ensure that script initiated navigations are
+  // honored. Exceptions being navigations initiated by the user.
+  if (!m_isNavigationHandledByClient ||
+      UserGestureIndicator::processingUserGesture()) {

[Nate Chapin, 2017/01/19 21:02:59]

Is this user gesture check necessary to get the tests to pass? It looks
different than what blink does for internal navigations, which seems less than
ideal.

[ananta, 2017/01/19 21:38:08]

The user gesture flag is passed via the FrameHostMsg_BeginNavigation IPC to the
browser and is sent back via the FrameMsg_CommitNavigation IPC back to the
renderer. This is then set in the form of a scoped user gesture for the duration
of the NavigateInternal() function.

The UserGestureIndicator::processingUserGesture() checks the same flag. Added a
comment explaining this.

[Nate Chapin, 2017/01/19 23:33:50]

Right, I got that. I just don't immediately see why you need to check
processingUserGesture() here. There's no similar special case for
processingUserGesture() when starting navigations without PlzNaviagate, is
there?

[ananta, 2017/01/20 00:00:33]

Based on our discussion, removing this check

+    m_frame->navigationScheduler().cancel();
+  }
   m_frame->editor().clearLastEditCommand();
 
   // If we are still in the process of initializing an empty document then its
   // frame is not in a consistent state for rendering, so avoid
   // setJSStatusBarText since it may cause clients to attempt to render the
   // frame.
   if (!m_stateMachine.creatingInitialEmptyDocument()) {
     DOMWindow* window = m_frame->domWindow();
     window->setStatus(String());
     window->setDefaultStatus(String());
@@ skipping 308 matching lines @@
   resourceRequest.setRequestContext(
       determineRequestContextFromNavigationType(navigationType));
   resourceRequest.setFrameType(m_frame->isMainFrame()
                                    ? WebURLRequest::FrameTypeTopLevel
                                    : WebURLRequest::FrameTypeNested);
 
   // Record the latest requiredCSP value that will be used when sending this
   // request.
   recordLatestRequiredCSP();
   modifyRequestForCSP(resourceRequest, nullptr);
+
+  bool navigationHandledByClient = m_isNavigationHandledByClient;

[Nate Chapin, 2017/01/19 21:02:59]

This should be probably have a "was" in the name, and note that
shouldContinueForNavigationPolicy may change its value?

[ananta, 2017/01/19 21:38:08]

Done.

+
   if (!shouldContinueForNavigationPolicy(
           resourceRequest, frameLoadRequest.substituteData(), nullptr,
           frameLoadRequest.shouldCheckMainWorldContentSecurityPolicy(),
           navigationType, navigationPolicy,
           type == FrameLoadTypeReplaceCurrentItem,
           frameLoadRequest.clientRedirect() ==
               ClientRedirectPolicy::ClientRedirect,
           frameLoadRequest.form()))
     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;
 
   m_provisionalDocumentLoader = client()->createDocumentLoader(
       m_frame, resourceRequest,
       frameLoadRequest.substituteData().isValid()
           ? frameLoadRequest.substituteData()
           : defaultSubstituteDataForURL(resourceRequest.url()),
       frameLoadRequest.clientRedirect());
   m_provisionalDocumentLoader->setNavigationType(navigationType);
   m_provisionalDocumentLoader->setReplacesCurrentHistoryItem(
       type == FrameLoadTypeReplaceCurrentItem);
-  m_frame->navigationScheduler().cancel();
-  m_checkTimer.stop();
+
+  // PlzNavigate: We need to ensure that script initiated navigations are
+  // honored. Exceptions being navigations initiated by the user.
+  if (!navigationHandledByClient ||
+      UserGestureIndicator::processingUserGesture()) {
+    m_frame->navigationScheduler().cancel();
+    m_checkTimer.stop();
+  }
 
   m_loadType = type;
 
   if (frameLoadRequest.form())
     client()->dispatchWillSubmitForm(frameLoadRequest.form());
 
   m_progressTracker->progressStarted();
   m_provisionalDocumentLoader->appendRedirect(
       m_provisionalDocumentLoader->getRequest().url());
   client()->dispatchDidStartProvisionalLoad();
@@ skipping 180 matching lines @@
                          m_documentLoader ? m_documentLoader->url() : String());
   return tracedValue;
 }
 
 inline void FrameLoader::takeObjectSnapshot() const {
   TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID("loading", "FrameLoader", this,
                                       toTracedValue());
 }
 
 }  // namespace blink