Title: [251333] branches/safari-608-branch
Revision
251333
Author
bshaf...@apple.com
Date
2019-10-20 10:32:36 -0700 (Sun, 20 Oct 2019)

Log Message

Cherry-pick r250416. rdar://problem/55954224

    SubFrameSOAuthorizationSession should ensure messages are posted in the right order to the parent frame
    https://bugs.webkit.org/show_bug.cgi?id=202061
    <rdar://problem/55485666>

    Reviewed by Youenn Fablet.

    Source/WebKit:

    This patch ensures messages that signal the process of SOAuthorization interception are posted in
    the right order to the parent frame. Before this patch, there are chances that SOAuthorizationDidCancel
    could be posted to the parent before SOAuthorizationDidStart. There are few causes that lead to
    this race condition:
    1) SubFrameSOAuthorizationSession::beforeStart posts SOAuthorizationDidStart in the next runloop. So
    extension could have the chance to invoke SubFrameSOAuthorizationSession::fallBackToWebPathInternal
    before SOAuthorizationDidStart is posted.
    2) Even if the order is right in the UI process, it is not guaranteed that Web process will strictly
    follow the order as the loading process is async.

    To fix the issue:
    1) SubFrameSOAuthorizationSession::beforeStart now posts SOAuthorizationDidStart in the same runloop.
    2) Observer is introduced in FrameLoadState such that SubFrameSOAuthorizationSession could know if
    the loading is finished. With this new capacity, SubFrameSOAuthorizationSession can ensure it only
    posts next message when the previous message has been posted.

    Implementation wise, a deque to queue requests is provided to maintain order.
    1) When new request is added to the deque, SubFrameSOAuthorizationSession will only load the request
    if it is the only element in the deque. Otherwise, it does nothing.
    2) When SubFrameSOAuthorizationSession receives didFinishLoad, it pops the head of the queue and loads
    the next request in the queue if any.
    The above design should guarantee all requests are loaded in sequence.

    * UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h:
    * UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm:
    (WebKit::SubFrameSOAuthorizationSession::SubFrameSOAuthorizationSession):
    (WebKit::SubFrameSOAuthorizationSession::~SubFrameSOAuthorizationSession):
    (WebKit::SubFrameSOAuthorizationSession::fallBackToWebPathInternal):
    (WebKit::SubFrameSOAuthorizationSession::completeInternal):
    (WebKit::SubFrameSOAuthorizationSession::beforeStart):
    (WebKit::SubFrameSOAuthorizationSession::didFinishLoad):
    (WebKit::SubFrameSOAuthorizationSession::appendRequestToLoad):
    (WebKit::SubFrameSOAuthorizationSession::loadRequestToFrame):
    (WebKit::SubFrameSOAuthorizationSession::loadDataToFrame): Deleted.
    (WebKit::SubFrameSOAuthorizationSession::postDidCancelMessageToParent): Deleted.
    * UIProcess/FrameLoadState.cpp:
    (WebKit::FrameLoadState::addObserver):
    (WebKit::FrameLoadState::removeObserver):
    (WebKit::FrameLoadState::didFinishLoad):
    * UIProcess/FrameLoadState.h:

    Tools:

    Adds tests that check the order of messages posted by SubFrameSOAuthorizationSession.

    * TestWebKitAPI/Tests/WebKitCocoa/TestSOAuthorization.mm:
    (-[TestSOAuthorizationScriptMessageHandler userContentController:didReceiveScriptMessage:]):
    (resetState):
    (TestWebKitAPI::TEST):

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

Modified Paths

Diff

Modified: branches/safari-608-branch/Source/WebKit/ChangeLog (251332 => 251333)


--- branches/safari-608-branch/Source/WebKit/ChangeLog	2019-10-20 17:32:33 UTC (rev 251332)
+++ branches/safari-608-branch/Source/WebKit/ChangeLog	2019-10-20 17:32:36 UTC (rev 251333)
@@ -1,3 +1,117 @@
+2019-10-15  Kocsen Chung  <kocsen_ch...@apple.com>
+
+        Cherry-pick r250416. rdar://problem/55954224
+
+    SubFrameSOAuthorizationSession should ensure messages are posted in the right order to the parent frame
+    https://bugs.webkit.org/show_bug.cgi?id=202061
+    <rdar://problem/55485666>
+    
+    Reviewed by Youenn Fablet.
+    
+    Source/WebKit:
+    
+    This patch ensures messages that signal the process of SOAuthorization interception are posted in
+    the right order to the parent frame. Before this patch, there are chances that SOAuthorizationDidCancel
+    could be posted to the parent before SOAuthorizationDidStart. There are few causes that lead to
+    this race condition:
+    1) SubFrameSOAuthorizationSession::beforeStart posts SOAuthorizationDidStart in the next runloop. So
+    extension could have the chance to invoke SubFrameSOAuthorizationSession::fallBackToWebPathInternal
+    before SOAuthorizationDidStart is posted.
+    2) Even if the order is right in the UI process, it is not guaranteed that Web process will strictly
+    follow the order as the loading process is async.
+    
+    To fix the issue:
+    1) SubFrameSOAuthorizationSession::beforeStart now posts SOAuthorizationDidStart in the same runloop.
+    2) Observer is introduced in FrameLoadState such that SubFrameSOAuthorizationSession could know if
+    the loading is finished. With this new capacity, SubFrameSOAuthorizationSession can ensure it only
+    posts next message when the previous message has been posted.
+    
+    Implementation wise, a deque to queue requests is provided to maintain order.
+    1) When new request is added to the deque, SubFrameSOAuthorizationSession will only load the request
+    if it is the only element in the deque. Otherwise, it does nothing.
+    2) When SubFrameSOAuthorizationSession receives didFinishLoad, it pops the head of the queue and loads
+    the next request in the queue if any.
+    The above design should guarantee all requests are loaded in sequence.
+    
+    * UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h:
+    * UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm:
+    (WebKit::SubFrameSOAuthorizationSession::SubFrameSOAuthorizationSession):
+    (WebKit::SubFrameSOAuthorizationSession::~SubFrameSOAuthorizationSession):
+    (WebKit::SubFrameSOAuthorizationSession::fallBackToWebPathInternal):
+    (WebKit::SubFrameSOAuthorizationSession::completeInternal):
+    (WebKit::SubFrameSOAuthorizationSession::beforeStart):
+    (WebKit::SubFrameSOAuthorizationSession::didFinishLoad):
+    (WebKit::SubFrameSOAuthorizationSession::appendRequestToLoad):
+    (WebKit::SubFrameSOAuthorizationSession::loadRequestToFrame):
+    (WebKit::SubFrameSOAuthorizationSession::loadDataToFrame): Deleted.
+    (WebKit::SubFrameSOAuthorizationSession::postDidCancelMessageToParent): Deleted.
+    * UIProcess/FrameLoadState.cpp:
+    (WebKit::FrameLoadState::addObserver):
+    (WebKit::FrameLoadState::removeObserver):
+    (WebKit::FrameLoadState::didFinishLoad):
+    * UIProcess/FrameLoadState.h:
+    
+    Tools:
+    
+    Adds tests that check the order of messages posted by SubFrameSOAuthorizationSession.
+    
+    * TestWebKitAPI/Tests/WebKitCocoa/TestSOAuthorization.mm:
+    (-[TestSOAuthorizationScriptMessageHandler userContentController:didReceiveScriptMessage:]):
+    (resetState):
+    (TestWebKitAPI::TEST):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250416 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-09-27  Jiewen Tan  <jiewen_...@apple.com>
+
+            SubFrameSOAuthorizationSession should ensure messages are posted in the right order to the parent frame
+            https://bugs.webkit.org/show_bug.cgi?id=202061
+            <rdar://problem/55485666>
+
+            Reviewed by Youenn Fablet.
+
+            This patch ensures messages that signal the process of SOAuthorization interception are posted in
+            the right order to the parent frame. Before this patch, there are chances that SOAuthorizationDidCancel
+            could be posted to the parent before SOAuthorizationDidStart. There are few causes that lead to
+            this race condition:
+            1) SubFrameSOAuthorizationSession::beforeStart posts SOAuthorizationDidStart in the next runloop. So
+            extension could have the chance to invoke SubFrameSOAuthorizationSession::fallBackToWebPathInternal
+            before SOAuthorizationDidStart is posted.
+            2) Even if the order is right in the UI process, it is not guaranteed that Web process will strictly
+            follow the order as the loading process is async.
+
+            To fix the issue:
+            1) SubFrameSOAuthorizationSession::beforeStart now posts SOAuthorizationDidStart in the same runloop.
+            2) Observer is introduced in FrameLoadState such that SubFrameSOAuthorizationSession could know if
+            the loading is finished. With this new capacity, SubFrameSOAuthorizationSession can ensure it only
+            posts next message when the previous message has been posted.
+
+            Implementation wise, a deque to queue requests is provided to maintain order.
+            1) When new request is added to the deque, SubFrameSOAuthorizationSession will only load the request
+            if it is the only element in the deque. Otherwise, it does nothing.
+            2) When SubFrameSOAuthorizationSession receives didFinishLoad, it pops the head of the queue and loads
+            the next request in the queue if any.
+            The above design should guarantee all requests are loaded in sequence.
+
+            * UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h:
+            * UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm:
+            (WebKit::SubFrameSOAuthorizationSession::SubFrameSOAuthorizationSession):
+            (WebKit::SubFrameSOAuthorizationSession::~SubFrameSOAuthorizationSession):
+            (WebKit::SubFrameSOAuthorizationSession::fallBackToWebPathInternal):
+            (WebKit::SubFrameSOAuthorizationSession::completeInternal):
+            (WebKit::SubFrameSOAuthorizationSession::beforeStart):
+            (WebKit::SubFrameSOAuthorizationSession::didFinishLoad):
+            (WebKit::SubFrameSOAuthorizationSession::appendRequestToLoad):
+            (WebKit::SubFrameSOAuthorizationSession::loadRequestToFrame):
+            (WebKit::SubFrameSOAuthorizationSession::loadDataToFrame): Deleted.
+            (WebKit::SubFrameSOAuthorizationSession::postDidCancelMessageToParent): Deleted.
+            * UIProcess/FrameLoadState.cpp:
+            (WebKit::FrameLoadState::addObserver):
+            (WebKit::FrameLoadState::removeObserver):
+            (WebKit::FrameLoadState::didFinishLoad):
+            * UIProcess/FrameLoadState.h:
+
 2019-10-04  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r250438. rdar://problem/55984974

Modified: branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SOAuthorizationCoordinator.mm (251332 => 251333)


--- branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SOAuthorizationCoordinator.mm	2019-10-20 17:32:33 UTC (rev 251332)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SOAuthorizationCoordinator.mm	2019-10-20 17:32:36 UTC (rev 251333)
@@ -28,6 +28,7 @@
 
 #if HAVE(APP_SSO)
 
+#import "APIFrameHandle.h"
 #import "PopUpSOAuthorizationSession.h"
 #import "RedirectSOAuthorizationSession.h"
 #import "SubFrameSOAuthorizationSession.h"
@@ -66,13 +67,14 @@
     }
 
     // SubFrameSOAuthorizationSession should only be allowed for Apple first parties.
-    bool subframeNavigation = navigationAction->targetFrame() && !navigationAction->targetFrame()->isMainFrame();
+    auto* targetFrame = navigationAction->targetFrame();
+    bool subframeNavigation = targetFrame && !targetFrame->isMainFrame();
     if (subframeNavigation && (!page.mainFrame() || ![AKAuthorizationController isURLFromAppleOwnedDomain:page.mainFrame()->url()])) {
         completionHandler(false);
         return;
     }
 
-    auto session = subframeNavigation ? SubFrameSOAuthorizationSession::create(m_soAuthorization.get(), WTFMove(navigationAction), page, WTFMove(completionHandler)) : RedirectSOAuthorizationSession::create(m_soAuthorization.get(), WTFMove(navigationAction), page, WTFMove(completionHandler));
+    auto session = subframeNavigation ? SubFrameSOAuthorizationSession::create(m_soAuthorization.get(), WTFMove(navigationAction), page, WTFMove(completionHandler), targetFrame->handle().frameID()) : RedirectSOAuthorizationSession::create(m_soAuthorization.get(), WTFMove(navigationAction), page, WTFMove(completionHandler));
     [m_soAuthorizationDelegate setSession:WTFMove(session)];
 }
 

Modified: branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h (251332 => 251333)


--- branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h	2019-10-20 17:32:33 UTC (rev 251332)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h	2019-10-20 17:32:36 UTC (rev 251333)
@@ -27,6 +27,7 @@
 
 #if HAVE(APP_SSO)
 
+#include "FrameLoadState.h"
 #include "NavigationSOAuthorizationSession.h"
 
 namespace IPC {
@@ -35,15 +36,21 @@
 
 namespace WebKit {
 
-class SubFrameSOAuthorizationSession final : public NavigationSOAuthorizationSession {
+class SubFrameSOAuthorizationSession final : public NavigationSOAuthorizationSession, public FrameLoadState::Observer {
 public:
     using Callback = CompletionHandler<void(bool)>;
+    using SOAuthorizationSession::weakPtrFactory;
+    using WeakValueType = SOAuthorizationSession::WeakValueType;
 
-    static Ref<SOAuthorizationSession> create(SOAuthorization *soAuthorization, Ref<API::NavigationAction>&& navigationAction, WebPageProxy& page, Callback&& completionHandler);
+    static Ref<SOAuthorizationSession> create(SOAuthorization *, Ref<API::NavigationAction>&&, WebPageProxy&, Callback&&, WebCore::FrameIdentifier);
 
+    ~SubFrameSOAuthorizationSession();
+
 private:
-    SubFrameSOAuthorizationSession(SOAuthorization *, Ref<API::NavigationAction>&&, WebPageProxy&, Callback&&);
+    using Supplement = Variant<Vector<uint8_t>, String>;
 
+    SubFrameSOAuthorizationSession(SOAuthorization *, Ref<API::NavigationAction>&&, WebPageProxy&, Callback&&, WebCore::FrameIdentifier);
+
     // SOAuthorizationSession
     void fallBackToWebPathInternal() final;
     void abortInternal() final;
@@ -52,8 +59,14 @@
     // NavigationSOAuthorizationSession
     void beforeStart() final;
 
-    void loadDataToFrame(const IPC::DataReference&, const URL&);
-    void postDidCancelMessageToParent(Function<void()>&&);
+    // FrameLoadState::Observer
+    void didFinishLoad() final;
+
+    void appendRequestToLoad(URL&&, Supplement&&);
+    void loadRequestToFrame();
+
+    WebCore::FrameIdentifier m_frameID;
+    Deque<std::pair<URL, Supplement>> m_requestsToLoad;
 };
 
 } // namespace WebKit

Modified: branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm (251332 => 251333)


--- branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm	2019-10-20 17:32:33 UTC (rev 251332)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm	2019-10-20 17:32:36 UTC (rev 251333)
@@ -28,7 +28,6 @@
 
 #if HAVE(APP_SSO)
 
-#import "APIFrameHandle.h"
 #import "APINavigationAction.h"
 #import "DataReference.h"
 #import "WebPageProxy.h"
@@ -39,39 +38,47 @@
 namespace WebKit {
 using namespace WebCore;
 
+namespace {
+
 const char* soAuthorizationPostDidStartMessageToParent = "<script>parent.postMessage('SOAuthorizationDidStart', '*');</script>";
-const char* soAuthorizationPostDidCancelMessageToParent = "parent.postMessage('SOAuthorizationDidCancel', '*');";
+const char* soAuthorizationPostDidCancelMessageToParent = "<script>parent.postMessage('SOAuthorizationDidCancel', '*');</script>";
 
-Ref<SOAuthorizationSession> SubFrameSOAuthorizationSession::create(SOAuthorization *soAuthorization, Ref<API::NavigationAction>&& navigationAction, WebPageProxy& page, Callback&& completionHandler)
+static inline Vector<uint8_t> convertBytesToVector(const uint8_t byteArray[], const size_t length)
 {
-    return adoptRef(*new SubFrameSOAuthorizationSession(soAuthorization, WTFMove(navigationAction), page, WTFMove(completionHandler)));
+    Vector<uint8_t> result;
+    result.append(byteArray, length);
+    return result;
 }
 
-SubFrameSOAuthorizationSession::SubFrameSOAuthorizationSession(SOAuthorization *soAuthorization, Ref<API::NavigationAction>&& navigationAction, WebPageProxy& page, Callback&& completionHandler)
+} // namespace
+
+Ref<SOAuthorizationSession> SubFrameSOAuthorizationSession::create(SOAuthorization *soAuthorization, Ref<API::NavigationAction>&& navigationAction, WebPageProxy& page, Callback&& completionHandler, FrameIdentifier frameID)
+{
+    return adoptRef(*new SubFrameSOAuthorizationSession(soAuthorization, WTFMove(navigationAction), page, WTFMove(completionHandler), frameID));
+}
+
+SubFrameSOAuthorizationSession::SubFrameSOAuthorizationSession(SOAuthorization *soAuthorization, Ref<API::NavigationAction>&& navigationAction, WebPageProxy& page, Callback&& completionHandler, FrameIdentifier frameID)
     : NavigationSOAuthorizationSession(soAuthorization, WTFMove(navigationAction), page, InitiatingAction::SubFrame, WTFMove(completionHandler))
+    , m_frameID(frameID)
 {
+    if (auto* frame = page.process().webFrame(m_frameID))
+        frame->frameLoadState().addObserver(*this);
 }
 
+SubFrameSOAuthorizationSession::~SubFrameSOAuthorizationSession()
+{
+    auto* page = this->page();
+    if (!page)
+        return;
+    if (auto* frame = page->process().webFrame(m_frameID))
+        frame->frameLoadState().removeObserver(*this);
+}
+
 void SubFrameSOAuthorizationSession::fallBackToWebPathInternal()
 {
-    // Instead of issuing a load, we execute the _javascript_ directly. This provides us a callback
-    // to ensure the final load is issued after the message is posted.
-    postDidCancelMessageToParent([weakThis = makeWeakPtr(*this)] {
-        if (!weakThis)
-            return;
-        auto* page = weakThis->page();
-        auto* navigationActionPtr = weakThis->navigationAction();
-        if (!page || !navigationActionPtr)
-            return;
-
-        if (auto* targetFrame = navigationActionPtr->targetFrame()) {
-            if (auto* frame = page->process().webFrame(targetFrame->handle().frameID())) {
-                page->setShouldSuppressSOAuthorizationInNextNavigationPolicyDecision();
-                // Issue a new load to the original URL as the original load is aborted before start.
-                frame->loadURL(navigationActionPtr->request().url(), navigationActionPtr->request().httpReferrer());
-            }
-        }
-    });
+    ASSERT(navigationAction());
+    appendRequestToLoad(URL(navigationAction()->request().url()), convertBytesToVector(reinterpret_cast<const uint8_t*>(soAuthorizationPostDidCancelMessageToParent), strlen(soAuthorizationPostDidCancelMessageToParent)));
+    appendRequestToLoad(URL(navigationAction()->request().url()), String(navigationAction()->request().httpReferrer()));
 }
 
 void SubFrameSOAuthorizationSession::abortInternal()
@@ -84,7 +91,7 @@
         fallBackToWebPathInternal();
         return;
     }
-    loadDataToFrame(IPC::DataReference(reinterpret_cast<const uint8_t*>(data.bytes), data.length), response.url());
+    appendRequestToLoad(URL(response.url()), convertBytesToVector(reinterpret_cast<const uint8_t*>(data.bytes), data.length));
 }
 
 void SubFrameSOAuthorizationSession::beforeStart()
@@ -91,40 +98,43 @@
 {
     // Cancelled the current load before loading the data to post SOAuthorizationDidStart to the parent frame.
     invokeCallback(true);
-    // Currently in the middle of decidePolicyForNavigationAction, should start a new load after.
-    RunLoop::main().dispatch([weakThis = makeWeakPtr(*this)] {
-        if (!weakThis || !weakThis->page() || !weakThis->navigationAction())
-            return;
-        // Instead of executing the _javascript_ directly, issuing a load. This will set the origin properly.
-        weakThis->loadDataToFrame(IPC::DataReference(reinterpret_cast<const uint8_t*>(soAuthorizationPostDidStartMessageToParent), strlen(soAuthorizationPostDidStartMessageToParent)), weakThis->navigationAction()->request().url());
-    });
+    ASSERT(navigationAction());
+    appendRequestToLoad(URL(navigationAction()->request().url()), convertBytesToVector(reinterpret_cast<const uint8_t*>(soAuthorizationPostDidStartMessageToParent), strlen(soAuthorizationPostDidStartMessageToParent)));
 }
 
-void SubFrameSOAuthorizationSession::loadDataToFrame(const IPC::DataReference& data, const URL& baseURL)
+void SubFrameSOAuthorizationSession::didFinishLoad()
 {
     auto* page = this->page();
-    auto* navigationActionPtr = navigationAction();
-    if (!page || !navigationActionPtr)
+    if (!page)
         return;
+    auto* frame = page->process().webFrame(m_frameID);
+    ASSERT(frame);
+    if (m_requestsToLoad.isEmpty() || m_requestsToLoad.first().first != frame->url())
+        return;
+    m_requestsToLoad.takeFirst();
+    loadRequestToFrame();
+}
 
-    if (auto* targetFrame = navigationActionPtr->targetFrame()) {
-        if (auto* frame = page->process().webFrame(targetFrame->handle().frameID())) {
-            page->setShouldSuppressSOAuthorizationInNextNavigationPolicyDecision();
-            frame->loadData(data, "text/html", "UTF-8", baseURL);
-        }
-    }
+void SubFrameSOAuthorizationSession::appendRequestToLoad(URL&& url, Supplement&& supplement)
+{
+    m_requestsToLoad.append({ WTFMove(url), WTFMove(supplement) });
+    if (m_requestsToLoad.size() == 1)
+        loadRequestToFrame();
 }
 
-void SubFrameSOAuthorizationSession::postDidCancelMessageToParent(Function<void()>&& callback)
+void SubFrameSOAuthorizationSession::loadRequestToFrame()
 {
     auto* page = this->page();
-    auto* navigationActionPtr = navigationAction();
-    if (!page || !navigationActionPtr)
+    if (!page || m_requestsToLoad.isEmpty())
         return;
 
-    if (auto* targetFrame = navigationActionPtr->targetFrame()) {
-        page->runJavaScriptInFrame(targetFrame->handle().frameID(), soAuthorizationPostDidCancelMessageToParent, false, [callback = WTFMove(callback)] (API::SerializedScriptValue*, bool, const ExceptionDetails&, ScriptValueCallback::Error) {
-            callback();
+    if (auto* frame = page->process().webFrame(m_frameID)) {
+        page->setShouldSuppressSOAuthorizationInNextNavigationPolicyDecision();
+        auto& url = ""
+        WTF::switchOn(m_requestsToLoad.first().second, [&](const Vector<uint8_t>& data) {
+            frame->loadData(IPC::DataReference(data), "text/html", "UTF-8", url);
+        }, [&](const String& referrer) {
+            frame->loadURL(url, referrer);
         });
     }
 }

Modified: branches/safari-608-branch/Source/WebKit/UIProcess/FrameLoadState.cpp (251332 => 251333)


--- branches/safari-608-branch/Source/WebKit/UIProcess/FrameLoadState.cpp	2019-10-20 17:32:33 UTC (rev 251332)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/FrameLoadState.cpp	2019-10-20 17:32:36 UTC (rev 251333)
@@ -32,6 +32,18 @@
 {
 }
 
+void FrameLoadState::addObserver(Observer& observer)
+{
+    auto result = m_observers.add(observer);
+    ASSERT_UNUSED(result, result.isNewEntry);
+}
+
+void FrameLoadState::removeObserver(Observer& observer)
+{
+    auto result = m_observers.remove(observer);
+    ASSERT_UNUSED(result, result);
+}
+
 void FrameLoadState::didStartProvisionalLoad(const URL& url)
 {
     ASSERT(m_provisionalURL.isEmpty());
@@ -77,6 +89,12 @@
     ASSERT(m_provisionalURL.isEmpty());
 
     m_state = State::Finished;
+
+    Vector<Observer*> observersCopy;
+    for (auto& observer : m_observers)
+        observersCopy.append(&observer);
+    for (auto* observer : observersCopy)
+        observer->didFinishLoad();
 }
 
 void FrameLoadState::didFailLoad()

Modified: branches/safari-608-branch/Source/WebKit/UIProcess/FrameLoadState.h (251332 => 251333)


--- branches/safari-608-branch/Source/WebKit/UIProcess/FrameLoadState.h	2019-10-20 17:32:33 UTC (rev 251332)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/FrameLoadState.h	2019-10-20 17:32:36 UTC (rev 251333)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include <wtf/URL.h>
+#include <wtf/WeakHashSet.h>
 
 namespace WebKit {
 
@@ -39,6 +40,16 @@
         Finished
     };
 
+    class Observer : public CanMakeWeakPtr<Observer> {
+    public:
+        virtual ~Observer() = default;
+
+        virtual void didFinishLoad() = 0;
+    };
+
+    void addObserver(Observer&);
+    void removeObserver(Observer&);
+
     void didStartProvisionalLoad(const URL&);
     void didExplicitOpen(const URL&);
     void didReceiveServerRedirectForProvisionalLoad(const URL&);
@@ -64,6 +75,7 @@
     URL m_provisionalURL;
     URL m_unreachableURL;
     URL m_lastUnreachableURL;
+    WeakHashSet<Observer> m_observers;
 };
 
 } // namespace WebKit

Modified: branches/safari-608-branch/Tools/ChangeLog (251332 => 251333)


--- branches/safari-608-branch/Tools/ChangeLog	2019-10-20 17:32:33 UTC (rev 251332)
+++ branches/safari-608-branch/Tools/ChangeLog	2019-10-20 17:32:36 UTC (rev 251333)
@@ -1,5 +1,85 @@
 2019-10-15  Kocsen Chung  <kocsen_ch...@apple.com>
 
+        Cherry-pick r250416. rdar://problem/55954224
+
+    SubFrameSOAuthorizationSession should ensure messages are posted in the right order to the parent frame
+    https://bugs.webkit.org/show_bug.cgi?id=202061
+    <rdar://problem/55485666>
+    
+    Reviewed by Youenn Fablet.
+    
+    Source/WebKit:
+    
+    This patch ensures messages that signal the process of SOAuthorization interception are posted in
+    the right order to the parent frame. Before this patch, there are chances that SOAuthorizationDidCancel
+    could be posted to the parent before SOAuthorizationDidStart. There are few causes that lead to
+    this race condition:
+    1) SubFrameSOAuthorizationSession::beforeStart posts SOAuthorizationDidStart in the next runloop. So
+    extension could have the chance to invoke SubFrameSOAuthorizationSession::fallBackToWebPathInternal
+    before SOAuthorizationDidStart is posted.
+    2) Even if the order is right in the UI process, it is not guaranteed that Web process will strictly
+    follow the order as the loading process is async.
+    
+    To fix the issue:
+    1) SubFrameSOAuthorizationSession::beforeStart now posts SOAuthorizationDidStart in the same runloop.
+    2) Observer is introduced in FrameLoadState such that SubFrameSOAuthorizationSession could know if
+    the loading is finished. With this new capacity, SubFrameSOAuthorizationSession can ensure it only
+    posts next message when the previous message has been posted.
+    
+    Implementation wise, a deque to queue requests is provided to maintain order.
+    1) When new request is added to the deque, SubFrameSOAuthorizationSession will only load the request
+    if it is the only element in the deque. Otherwise, it does nothing.
+    2) When SubFrameSOAuthorizationSession receives didFinishLoad, it pops the head of the queue and loads
+    the next request in the queue if any.
+    The above design should guarantee all requests are loaded in sequence.
+    
+    * UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.h:
+    * UIProcess/Cocoa/SOAuthorization/SubFrameSOAuthorizationSession.mm:
+    (WebKit::SubFrameSOAuthorizationSession::SubFrameSOAuthorizationSession):
+    (WebKit::SubFrameSOAuthorizationSession::~SubFrameSOAuthorizationSession):
+    (WebKit::SubFrameSOAuthorizationSession::fallBackToWebPathInternal):
+    (WebKit::SubFrameSOAuthorizationSession::completeInternal):
+    (WebKit::SubFrameSOAuthorizationSession::beforeStart):
+    (WebKit::SubFrameSOAuthorizationSession::didFinishLoad):
+    (WebKit::SubFrameSOAuthorizationSession::appendRequestToLoad):
+    (WebKit::SubFrameSOAuthorizationSession::loadRequestToFrame):
+    (WebKit::SubFrameSOAuthorizationSession::loadDataToFrame): Deleted.
+    (WebKit::SubFrameSOAuthorizationSession::postDidCancelMessageToParent): Deleted.
+    * UIProcess/FrameLoadState.cpp:
+    (WebKit::FrameLoadState::addObserver):
+    (WebKit::FrameLoadState::removeObserver):
+    (WebKit::FrameLoadState::didFinishLoad):
+    * UIProcess/FrameLoadState.h:
+    
+    Tools:
+    
+    Adds tests that check the order of messages posted by SubFrameSOAuthorizationSession.
+    
+    * TestWebKitAPI/Tests/WebKitCocoa/TestSOAuthorization.mm:
+    (-[TestSOAuthorizationScriptMessageHandler userContentController:didReceiveScriptMessage:]):
+    (resetState):
+    (TestWebKitAPI::TEST):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250416 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-09-27  Jiewen Tan  <jiewen_...@apple.com>
+
+            SubFrameSOAuthorizationSession should ensure messages are posted in the right order to the parent frame
+            https://bugs.webkit.org/show_bug.cgi?id=202061
+            <rdar://problem/55485666>
+
+            Reviewed by Youenn Fablet.
+
+            Adds tests that check the order of messages posted by SubFrameSOAuthorizationSession.
+
+            * TestWebKitAPI/Tests/WebKitCocoa/TestSOAuthorization.mm:
+            (-[TestSOAuthorizationScriptMessageHandler userContentController:didReceiveScriptMessage:]):
+            (resetState):
+            (TestWebKitAPI::TEST):
+
+2019-10-15  Kocsen Chung  <kocsen_ch...@apple.com>
+
         Cherry-pick r249517. rdar://problem/56000099
 
     Mail appears to be double inverting code copied from Notes, Xcode, or Terminal.

Modified: branches/safari-608-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/TestSOAuthorization.mm (251332 => 251333)


--- branches/safari-608-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/TestSOAuthorization.mm	2019-10-20 17:32:33 UTC (rev 251332)
+++ branches/safari-608-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/TestSOAuthorization.mm	2019-10-20 17:32:36 UTC (rev 251333)
@@ -53,6 +53,7 @@
 static bool newWindowCreated = false;
 static bool haveHttpBody = false;
 static bool navigationPolicyDecided = false;
+static bool allMessagesReceived = false;
 static String finalURL;
 static SOAuthorization* gAuthorization;
 static id<SOAuthorizationDelegate> gDelegate;
@@ -241,6 +242,37 @@
 
 @end
 
+@interface TestSOAuthorizationScriptMessageHandler : NSObject <WKScriptMessageHandler>
+@end
+
+@implementation TestSOAuthorizationScriptMessageHandler {
+    RetainPtr<NSMutableArray> _messages;
+}
+
+- (void)userContentController:(WKUserContentController *)userContentController didReceiveScriptMessage:(WKScriptMessage *)message
+{
+    if (!_messages)
+        _messages = adoptNS([[NSMutableArray alloc] init]);
+    [_messages addObject:message.body];
+
+    if ([message.body isEqual:@""]) {
+        allMessagesReceived = true;
+        EXPECT_EQ([_messages count], 5u);
+        EXPECT_WK_STREQ("SOAuthorizationDidStart", [_messages objectAtIndex:1]);
+        EXPECT_WK_STREQ("SOAuthorizationDidCancel", [_messages objectAtIndex:3]);
+        EXPECT_WK_STREQ("", [_messages objectAtIndex:4]);
+    }
+
+    if ([message.body isEqual:@"Hello."]) {
+        allMessagesReceived = true;
+        EXPECT_EQ([_messages count], 4u);
+        EXPECT_WK_STREQ("SOAuthorizationDidStart", [_messages objectAtIndex:1]);
+        EXPECT_WK_STREQ("Hello.", [_messages objectAtIndex:3]);
+    }
+}
+
+@end
+
 static bool overrideCanPerformAuthorizationWithURL(id, SEL, NSURL *url, NSInteger)
 {
     if (![url.lastPathComponent isEqual:@"simple.html"] && ![url.host isEqual:@"www.example.com"] && ![url.lastPathComponent isEqual:@"GetSessionCookie.html"])
@@ -292,6 +324,7 @@
     newWindowCreated = false;
     haveHttpBody = false;
     navigationPolicyDecided = false;
+    allMessagesReceived = false;
     finalURL = emptyString();
     gAuthorization = nullptr;
     gDelegate = nullptr;
@@ -2220,6 +2253,60 @@
     [webView waitForMessage:(id)makeString("Referrer: ", origin, "/")]; // Referrer policy requires '/' after origin.
 }
 
+TEST(SOAuthorizationSubFrame, InterceptionErrorMessageOrder)
+{
+    resetState();
+    ClassMethodSwizzler swizzler1(PAL::getSOAuthorizationClass(), @selector(canPerformAuthorizationWithURL:responseCode:), reinterpret_cast<IMP>(overrideCanPerformAuthorizationWithURL));
+    InstanceMethodSwizzler swizzler2(PAL::getSOAuthorizationClass(), @selector(setDelegate:), reinterpret_cast<IMP>(overrideSetDelegate));
+    InstanceMethodSwizzler swizzler3(PAL::getSOAuthorizationClass(), @selector(beginAuthorizationWithURL:httpHeaders:httpBody:), reinterpret_cast<IMP>(overrideBeginAuthorizationWithURL));
+    ClassMethodSwizzler swizzler4([AKAuthorizationController class], @selector(isURLFromAppleOwnedDomain:), reinterpret_cast<IMP>(overrideIsURLFromAppleOwnedDomain));
+
+    RetainPtr<NSURL> baseURL = [[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+    RetainPtr<NSURL> testURL = [[NSBundle mainBundle] URLForResource:@"GetSessionCookie" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+    auto testHtml = generateHtml(parentTemplate, testURL.get().absoluteString);
+
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto messageHandler = adoptNS([[TestSOAuthorizationScriptMessageHandler alloc] init]);
+    [[configuration userContentController] addScriptMessageHandler:messageHandler.get() name:@"testHandler"];
+
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration.get()]);
+    auto delegate = adoptNS([[TestSOAuthorizationDelegate alloc] init]);
+    configureSOAuthorizationWebView(webView.get(), delegate.get());
+
+    [webView loadHTMLString:testHtml baseURL:baseURL.get()];
+    Util::run(&authorizationPerformed);
+    [gDelegate authorization:gAuthorization didCompleteWithError:adoptNS([[NSError alloc] initWithDomain:NSCocoaErrorDomain code:0 userInfo:nil]).get()];
+    Util::run(&allMessagesReceived);
+}
+
+TEST(SOAuthorizationSubFrame, InterceptionSuccessMessageOrder)
+{
+    resetState();
+    ClassMethodSwizzler swizzler1(PAL::getSOAuthorizationClass(), @selector(canPerformAuthorizationWithURL:responseCode:), reinterpret_cast<IMP>(overrideCanPerformAuthorizationWithURL));
+    InstanceMethodSwizzler swizzler2(PAL::getSOAuthorizationClass(), @selector(setDelegate:), reinterpret_cast<IMP>(overrideSetDelegate));
+    InstanceMethodSwizzler swizzler3(PAL::getSOAuthorizationClass(), @selector(beginAuthorizationWithURL:httpHeaders:httpBody:), reinterpret_cast<IMP>(overrideBeginAuthorizationWithURL));
+    ClassMethodSwizzler swizzler4([AKAuthorizationController class], @selector(isURLFromAppleOwnedDomain:), reinterpret_cast<IMP>(overrideIsURLFromAppleOwnedDomain));
+
+    auto testURL = URL(URL(), "http://www.example.com");
+    auto testHtml = generateHtml(parentTemplate, testURL.string());
+
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    auto messageHandler = adoptNS([[TestSOAuthorizationScriptMessageHandler alloc] init]);
+    [[configuration userContentController] addScriptMessageHandler:messageHandler.get() name:@"testHandler"];
+
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration.get()]);
+    auto delegate = adoptNS([[TestSOAuthorizationDelegate alloc] init]);
+    configureSOAuthorizationWebView(webView.get(), delegate.get());
+
+    [webView loadHTMLString:testHtml baseURL:nil];
+    Util::run(&authorizationPerformed);
+
+    auto response = adoptNS([[NSHTTPURLResponse alloc] initWithURL:testURL statusCode:200 HTTPVersion:@"HTTP/1.1" headerFields:nil]);
+    auto iframeHtmlCString = generateHtml(iframeTemplate, "").utf8();
+    [gDelegate authorization:gAuthorization didCompleteWithHTTPResponse:response.get() httpBody:adoptNS([[NSData alloc] initWithBytes:iframeHtmlCString.data() length:iframeHtmlCString.length()]).get()];
+    Util::run(&allMessagesReceived);
+}
+
 } // namespace TestWebKitAPI
 
 #endif
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to