NavigationAction should track whether the navigation was initiated by the main frame
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Jul 2017 17:25:37 +0000 (17:25 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Jul 2017 17:25:37 +0000 (17:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174386
<rdar://problem/33245267>

Reviewed by Brady Eidson.

Source/WebCore:

Although we added state to NavigationAction to track whether the navigation was
initiated by the main frame in r219170 it is not possible to initialize this state
when instantiating a NavigationAction. Having NavigationAction track this state
will be useful to ensure that we can always compute the source frame information
when asking the embedding client whether to allow a navigation. We will make use
of it in the fix for <https://bugs.webkit.org/show_bug.cgi?id=174385>.

No behavior changed. So, no new tests.

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadURL): Pass whether the load was initiated by the main frame
when instantiating the NavigationAction.
(WebCore::FrameLoader::load): For now, pass InitiatedByMainFrame::Unknown when instantiating
the NavigationAction as we do not know if the load was initiated by the main frame.
(WebCore::FrameLoader::loadWithDocumentLoader): Ditto.
(WebCore::FrameLoader::reload): Ditto
(WebCore::FrameLoader::loadDifferentDocumentItem): Ditto.
(WebCore::createWindow): Pass whether the load was initiated by the main frame when
instantiating the NavigationAction.
* loader/NavigationAction.cpp:
(WebCore::NavigationAction::NavigationAction): Modified to take argument of type InitiatedByMainFrame
that indicates whether the navigation was initiated by the main frame.
* loader/NavigationAction.h:
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy): For now, pass InitiatedByMainFrame::Unknown
when instantiating the NavigationAction as we do not know if the load was initiated by the
main frame.
* page/ContextMenuController.cpp:
(WebCore::openNewWindow): Pass whether the load was initiated by the main frame when
instantiating the NavigationAction.

Source/WebKit2:

* WebProcess/WebPage/WebInspector.cpp:
(WebKit::WebInspector::openInNewTab): Pass whether the load was initiated by the main frame
when instantiating the NavigationAction.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@219407 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/NavigationAction.cpp
Source/WebCore/loader/NavigationAction.h
Source/WebCore/loader/PolicyChecker.cpp
Source/WebCore/page/ContextMenuController.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/WebInspector.cpp

index e77a0a0..986bc7d 100644 (file)
@@ -1,5 +1,44 @@
 2017-07-12  Daniel Bates  <dabates@apple.com>
 
+        NavigationAction should track whether the navigation was initiated by the main frame
+        https://bugs.webkit.org/show_bug.cgi?id=174386
+        <rdar://problem/33245267>
+
+        Reviewed by Brady Eidson.
+
+        Although we added state to NavigationAction to track whether the navigation was
+        initiated by the main frame in r219170 it is not possible to initialize this state
+        when instantiating a NavigationAction. Having NavigationAction track this state
+        will be useful to ensure that we can always compute the source frame information
+        when asking the embedding client whether to allow a navigation. We will make use
+        of it in the fix for <https://bugs.webkit.org/show_bug.cgi?id=174385>.
+
+        No behavior changed. So, no new tests.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadURL): Pass whether the load was initiated by the main frame
+        when instantiating the NavigationAction.
+        (WebCore::FrameLoader::load): For now, pass InitiatedByMainFrame::Unknown when instantiating
+        the NavigationAction as we do not know if the load was initiated by the main frame.
+        (WebCore::FrameLoader::loadWithDocumentLoader): Ditto.
+        (WebCore::FrameLoader::reload): Ditto
+        (WebCore::FrameLoader::loadDifferentDocumentItem): Ditto.
+        (WebCore::createWindow): Pass whether the load was initiated by the main frame when
+        instantiating the NavigationAction.
+        * loader/NavigationAction.cpp:
+        (WebCore::NavigationAction::NavigationAction): Modified to take argument of type InitiatedByMainFrame
+        that indicates whether the navigation was initiated by the main frame.
+        * loader/NavigationAction.h:
+        * loader/PolicyChecker.cpp:
+        (WebCore::PolicyChecker::checkNavigationPolicy): For now, pass InitiatedByMainFrame::Unknown
+        when instantiating the NavigationAction as we do not know if the load was initiated by the
+        main frame.
+        * page/ContextMenuController.cpp:
+        (WebCore::openNewWindow): Pass whether the load was initiated by the main frame when
+        instantiating the NavigationAction.
+
+2017-07-12  Daniel Bates  <dabates@apple.com>
+
         Rename NavigationInitiatedByMainFrame to InitiatedByMainFrame
         https://bugs.webkit.org/show_bug.cgi?id=174427
 
index 89eeb8d..1b70297 100644 (file)
@@ -1289,7 +1289,7 @@ void FrameLoader::loadURL(FrameLoadRequest&& frameLoadRequest, const String& ref
     if (!isNavigationAllowed())
         return;
 
-    NavigationAction action { frameLoadRequest.requester(), request, newLoadType, isFormSubmission, event, frameLoadRequest.shouldOpenExternalURLsPolicy(), frameLoadRequest.downloadAttribute() };
+    NavigationAction action { frameLoadRequest.requester(), request, frameLoadRequest.initiatedByMainFrame(), newLoadType, isFormSubmission, event, frameLoadRequest.shouldOpenExternalURLsPolicy(), frameLoadRequest.downloadAttribute() };
 
     if (!targetFrame && !frameName.isEmpty()) {
         action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, frameLoadRequest));
@@ -1362,7 +1362,7 @@ void FrameLoader::load(FrameLoadRequest&& request)
     }
 
     if (request.shouldCheckNewWindowPolicy()) {
-        NavigationAction action { request.requester(), request.resourceRequest(), NavigationType::Other, request.shouldOpenExternalURLsPolicy() };
+        NavigationAction action { request.requester(), request.resourceRequest(), InitiatedByMainFrame::Unknown, NavigationType::Other, request.shouldOpenExternalURLsPolicy() };
         policyChecker().checkNewWindowPolicy(action, request.resourceRequest(), nullptr, request.frameName(), [this] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
             continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
         });
@@ -1467,7 +1467,7 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
 
     if (shouldPerformFragmentNavigation(isFormSubmission, httpMethod, policyChecker().loadType(), newURL)) {
         RefPtr<DocumentLoader> oldDocumentLoader = m_documentLoader;
-        NavigationAction action { *m_frame.document(), loader->request(), policyChecker().loadType(), isFormSubmission };
+        NavigationAction action { *m_frame.document(), loader->request(), InitiatedByMainFrame::Unknown, policyChecker().loadType(), isFormSubmission };
 
         oldDocumentLoader->setTriggeringAction(action);
         oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
@@ -1484,7 +1484,7 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
     policyChecker().stopCheck();
     setPolicyDocumentLoader(loader);
     if (loader->triggeringAction().isEmpty())
-        loader->setTriggeringAction({ *m_frame.document(), loader->request(), policyChecker().loadType(), isFormSubmission });
+        loader->setTriggeringAction({ *m_frame.document(), loader->request(), InitiatedByMainFrame::Unknown, policyChecker().loadType(), isFormSubmission });
 
     if (Element* ownerElement = m_frame.ownerElement()) {
         // We skip dispatching the beforeload event if we've already
@@ -1623,7 +1623,7 @@ void FrameLoader::reload(OptionSet<ReloadOption> options)
 
     // If we're about to re-post, set up action so the application can warn the user.
     if (request.httpMethod() == "POST")
-        loader->setTriggeringAction({ *m_frame.document(), request, NavigationType::FormResubmitted });
+        loader->setTriggeringAction({ *m_frame.document(), request, InitiatedByMainFrame::Unknown, NavigationType::FormResubmitted });
 
     loader->setOverrideEncoding(m_documentLoader->overrideEncoding());
 
@@ -3386,10 +3386,12 @@ void FrameLoader::loadDifferentDocumentItem(HistoryItem& item, FrameLoadType loa
     // Remember this item so we can traverse any child items as child frames load
     history().setProvisionalItem(&item);
 
+    auto initiatedByMainFrame = InitiatedByMainFrame::Unknown;
+
     if (CachedPage* cachedPage = PageCache::singleton().get(item, m_frame.page())) {
         auto documentLoader = cachedPage->documentLoader();
         m_client.updateCachedDocumentLoader(*documentLoader);
-        documentLoader->setTriggeringAction({ *m_frame.document(), documentLoader->request(), loadType, false });
+        documentLoader->setTriggeringAction({ *m_frame.document(), documentLoader->request(), initiatedByMainFrame, loadType, false });
         documentLoader->setLastCheckedRequest(ResourceRequest());
         loadWithDocumentLoader(documentLoader, loadType, 0, AllowNavigationToInvalidURL::Yes);
         return;
@@ -3437,10 +3439,10 @@ void FrameLoader::loadDifferentDocumentItem(HistoryItem& item, FrameLoadType loa
         
         if (cacheLoadPolicy == MayAttemptCacheOnlyLoadForFormSubmissionItem) {
             request.setCachePolicy(ReturnCacheDataDontLoad);
-            action = { *m_frame.document(), request, loadType, isFormSubmission, event, shouldOpenExternalURLsPolicy };
+            action = { *m_frame.document(), request, initiatedByMainFrame, loadType, isFormSubmission, event, shouldOpenExternalURLsPolicy };
         } else {
             request.setCachePolicy(ReturnCacheDataElseLoad);
-            action = { *m_frame.document(), request, NavigationType::FormResubmitted, shouldOpenExternalURLsPolicy, event };
+            action = { *m_frame.document(), request, initiatedByMainFrame, NavigationType::FormResubmitted, shouldOpenExternalURLsPolicy, event };
         }
     } else {
         switch (loadType) {
@@ -3723,7 +3725,7 @@ RefPtr<Frame> createWindow(Frame& openerFrame, Frame& lookupFrame, FrameLoadRequ
         return nullptr;
 
     ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy = shouldOpenExternalURLsPolicyToApply(openerFrame, request);
-    NavigationAction action { request.requester(), request.resourceRequest(), NavigationType::Other, shouldOpenExternalURLsPolicy };
+    NavigationAction action { request.requester(), request.resourceRequest(), request.initiatedByMainFrame(), NavigationType::Other, shouldOpenExternalURLsPolicy };
     Page* page = oldPage->chrome().createWindow(openerFrame, request, features, action);
     if (!page)
         return nullptr;
index 8b029d4..7514b34 100644 (file)
@@ -44,11 +44,12 @@ NavigationAction::NavigationAction(NavigationAction&&) = default;
 NavigationAction& NavigationAction::operator=(const NavigationAction&) = default;
 NavigationAction& NavigationAction::operator=(NavigationAction&&) = default;
 
-NavigationAction::NavigationAction(Document& source, const ResourceRequest& resourceRequest, NavigationType type, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, Event* event, const AtomicString& downloadAttribute)
+NavigationAction::NavigationAction(Document& source, const ResourceRequest& resourceRequest, InitiatedByMainFrame initiatedByMainFrame, NavigationType type, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, Event* event, const AtomicString& downloadAttribute)
     : m_sourceDocument { makeRefPtr(source) }
     , m_resourceRequest { resourceRequest }
     , m_type { type }
     , m_shouldOpenExternalURLsPolicy { shouldOpenExternalURLsPolicy }
+    , m_initiatedByMainFrame { initiatedByMainFrame }
     , m_event { event }
     , m_downloadAttribute { downloadAttribute }
 {
@@ -67,11 +68,12 @@ static NavigationType navigationType(FrameLoadType frameLoadType, bool isFormSub
     return NavigationType::Other;
 }
 
-NavigationAction::NavigationAction(Document& source, const ResourceRequest& resourceRequest, FrameLoadType frameLoadType, bool isFormSubmission, Event* event, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, const AtomicString& downloadAttribute)
+NavigationAction::NavigationAction(Document& source, const ResourceRequest& resourceRequest, InitiatedByMainFrame initiatedByMainFrame, FrameLoadType frameLoadType, bool isFormSubmission, Event* event, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, const AtomicString& downloadAttribute)
     : m_sourceDocument { makeRefPtr(source) }
     , m_resourceRequest { resourceRequest }
     , m_type { navigationType(frameLoadType, isFormSubmission, !!event) }
     , m_shouldOpenExternalURLsPolicy { shouldOpenExternalURLsPolicy }
+    , m_initiatedByMainFrame { initiatedByMainFrame }
     , m_event { event }
     , m_downloadAttribute { downloadAttribute }
 {
index 0bab56a..86be91a 100644 (file)
@@ -41,8 +41,8 @@ class Event;
 class NavigationAction {
 public:
     NavigationAction();
-    WEBCORE_EXPORT NavigationAction(Document&, const ResourceRequest&, NavigationType = NavigationType::Other, ShouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow, Event* = nullptr, const AtomicString& downloadAttribute = nullAtom());
-    NavigationAction(Document&, const ResourceRequest&, FrameLoadType, bool isFormSubmission, Event* = nullptr, ShouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow, const AtomicString& downloadAttribute = nullAtom());
+    WEBCORE_EXPORT NavigationAction(Document&, const ResourceRequest&, InitiatedByMainFrame, NavigationType = NavigationType::Other, ShouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow, Event* = nullptr, const AtomicString& downloadAttribute = nullAtom());
+    NavigationAction(Document&, const ResourceRequest&, InitiatedByMainFrame, FrameLoadType, bool isFormSubmission, Event* = nullptr, ShouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow, const AtomicString& downloadAttribute = nullAtom());
 
     WEBCORE_EXPORT ~NavigationAction();
 
index cf2ea32..5c68975 100644 (file)
@@ -84,7 +84,7 @@ void PolicyChecker::checkNavigationPolicy(const ResourceRequest& request, bool d
 {
     NavigationAction action = loader->triggeringAction();
     if (action.isEmpty()) {
-        action = NavigationAction { *m_frame.document(), request, NavigationType::Other, loader->shouldOpenExternalURLsPolicyToPropagate() };
+        action = NavigationAction { *m_frame.document(), request, InitiatedByMainFrame::Unknown, NavigationType::Other, loader->shouldOpenExternalURLsPolicyToPropagate() };
         loader->setTriggeringAction(action);
     }
 
index 9884a22..8011c42 100644 (file)
@@ -191,7 +191,7 @@ static void openNewWindow(const URL& urlToLoad, Frame& frame, ShouldOpenExternal
 
     FrameLoadRequest frameLoadRequest { *frame.document(), frame.document()->securityOrigin(), ResourceRequest(urlToLoad, frame.loader().outgoingReferrer()), { }, LockHistory::No, LockBackForwardList::No, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress, shouldOpenExternalURLsPolicy, InitiatedByMainFrame::Unknown };
 
-    Page* newPage = oldPage->chrome().createWindow(frame, frameLoadRequest, { }, { *frame.document(), frameLoadRequest.resourceRequest() });
+    Page* newPage = oldPage->chrome().createWindow(frame, frameLoadRequest, { }, { *frame.document(), frameLoadRequest.resourceRequest(), frameLoadRequest.initiatedByMainFrame() });
     if (!newPage)
         return;
     newPage->chrome().show();
index 803dc23..5ec63f1 100644 (file)
@@ -1,5 +1,17 @@
 2017-07-12  Daniel Bates  <dabates@apple.com>
 
+        NavigationAction should track whether the navigation was initiated by the main frame
+        https://bugs.webkit.org/show_bug.cgi?id=174386
+        <rdar://problem/33245267>
+
+        Reviewed by Brady Eidson.
+
+        * WebProcess/WebPage/WebInspector.cpp:
+        (WebKit::WebInspector::openInNewTab): Pass whether the load was initiated by the main frame
+        when instantiating the NavigationAction.
+
+2017-07-12  Daniel Bates  <dabates@apple.com>
+
         Rename NavigationInitiatedByMainFrame to InitiatedByMainFrame
         https://bugs.webkit.org/show_bug.cgi?id=174427
 
index 2a23dd2..6a890b2 100644 (file)
@@ -149,7 +149,7 @@ void WebInspector::openInNewTab(const String& urlString)
     Frame& inspectedMainFrame = inspectedPage->mainFrame();
     FrameLoadRequest frameLoadRequest { *inspectedMainFrame.document(), inspectedMainFrame.document()->securityOrigin(), { urlString }, ASCIILiteral("_blank"), LockHistory::No, LockBackForwardList::No, MaybeSendReferrer, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Allow, ShouldOpenExternalURLsPolicy::ShouldNotAllow, InitiatedByMainFrame::Unknown };
 
-    NavigationAction action { *inspectedMainFrame.document(), frameLoadRequest.resourceRequest(), NavigationType::LinkClicked };
+    NavigationAction action { *inspectedMainFrame.document(), frameLoadRequest.resourceRequest(), frameLoadRequest.initiatedByMainFrame(), NavigationType::LinkClicked };
     Page* newPage = inspectedPage->chrome().createWindow(inspectedMainFrame, frameLoadRequest, { }, action);
     if (!newPage)
         return;