Attachment 358346 Details for Bug 193151 – Patch

[patch] Patch

bug-193151-20190104122154.patch (text/plain), 14.64 KB, created by Chris Dumez on 2019-01-04 12:21:54 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-04 12:21:54 PST

Size: 14.64 KB

patch

obsolete

>Subversion Revision: 239618
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9c6713e6cb81a05b27dd3a361c984d754082ef8b..c42470d8bf49862ce5e4c6110cf1b5da92400004 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-01-04  Chris Dumez  <cdumez@apple.com>
>+
>+        Add WKFrameInfo._hasHadUserInteraction SPI
>+        https://bugs.webkit.org/show_bug.cgi?id=193151
>+        <rdar://problem/36074736>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * bindings/js/ScriptController.cpp:
>+        (WebCore::ScriptController::executeScriptInWorld):
>+        Pass a document when constructing the UserGestureIndicator so that the hasHadUserInteraction
>+        flag properly gets set on the document / frame.
>+
>+        * dom/UserGestureIndicator.cpp:
>+        Do an early return when state is nullopt since no user interaction happens in this case and
>+        we do not want to run the rest of the method. The rest of the method used to be a no-op because
>+        document is normally nullptr when state is nullopt. However, I updated executeScriptInWorld()
>+        to pass a document even when state is not std::nullopt.
>+
>+        * page/Frame.h:
>+        Store a bit on the frame to indicate if the user has ever interacted with the frame.
>+
> 2019-01-04  Chris Fleizach  <cfleizach@apple.com>
> 
>         AX: String check: "Rule" does not reflect the meaning of the <hr> html tag
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index dddfdb019947a39a5268e186cf8153c643a3b2c4..b265cf2e1d7ea93a2a1b36d305ecb20930cc403b 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-01-04  Chris Dumez  <cdumez@apple.com>
>+
>+        Add WKFrameInfo._hasHadUserInteraction SPI
>+        https://bugs.webkit.org/show_bug.cgi?id=193151
>+        <rdar://problem/36074736>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add WKFrameInfo._hasHadUserInteraction SPI so that the client can know if
>+        the user has ever interacted with the frame.
>+
>+        * Shared/FrameInfoData.cpp:
>+        (WebKit::FrameInfoData::encode const):
>+        (WebKit::FrameInfoData::decode):
>+        * Shared/FrameInfoData.h:
>+        * UIProcess/API/APIFrameInfo.cpp:
>+        * UIProcess/API/APIFrameInfo.h:
>+        * UIProcess/API/Cocoa/WKFrameInfo.mm:
>+        (-[WKFrameInfo _hasHadUserInteraction]):
>+        * UIProcess/API/Cocoa/WKFrameInfoPrivate.h:
>+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
>+        (WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):
>+
> 2019-01-04  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r239603.
>diff --git a/Source/WebCore/bindings/js/ScriptController.cpp b/Source/WebCore/bindings/js/ScriptController.cpp
>index 785c66ec47a27a313b8cb0e4f2df836634dbe255..c2358ea1c650e8c91f9a02e5b38a308917ccf6b3 100644
>--- a/Source/WebCore/bindings/js/ScriptController.cpp
>+++ b/Source/WebCore/bindings/js/ScriptController.cpp
>@@ -542,7 +542,7 @@ void ScriptController::clearScriptObjects()
> 
> JSValue ScriptController::executeScriptInWorld(DOMWrapperWorld& world, const String& script, bool forceUserGesture, ExceptionDetails* exceptionDetails)
> {
>-    UserGestureIndicator gestureIndicator(forceUserGesture ? Optional<ProcessingUserGestureState>(ProcessingUserGesture) : WTF::nullopt);
>+    UserGestureIndicator gestureIndicator(forceUserGesture ? Optional<ProcessingUserGestureState>(ProcessingUserGesture) : WTF::nullopt, m_frame.document());
>     ScriptSourceCode sourceCode(script, URL(m_frame.document()->url()), TextPosition(), JSC::SourceProviderSourceType::Program, CachedScriptFetcher::create(m_frame.document()->charset()));
> 
>     if (!canExecuteScripts(AboutToExecuteScript) || isPaused())
>diff --git a/Source/WebCore/dom/UserGestureIndicator.cpp b/Source/WebCore/dom/UserGestureIndicator.cpp
>index d1685b9ae5f2c85349639de387340e99c4925b02..1ffbb6e0437f15854b3220a562b987925f643720 100644
>--- a/Source/WebCore/dom/UserGestureIndicator.cpp
>+++ b/Source/WebCore/dom/UserGestureIndicator.cpp
>@@ -27,6 +27,7 @@
> #include "UserGestureIndicator.h"
> 
> #include "Document.h"
>+#include "Frame.h"
> #include "ResourceLoadObserver.h"
> #include <wtf/MainThread.h>
> #include <wtf/NeverDestroyed.h>
>@@ -51,14 +52,18 @@ UserGestureIndicator::UserGestureIndicator(Optional<ProcessingUserGestureState>
> {
>     ASSERT(isMainThread());
> 
>-    if (state)
>-        currentToken() = UserGestureToken::create(state.value(), gestureType);
>+    if (!state)
>+        return;
>+
>+    currentToken() = UserGestureToken::create(state.value(), gestureType);
> 
>     if (document && currentToken()->processingUserGesture()) {
>         document->updateLastHandledUserGestureTimestamp(MonotonicTime::now());
>         if (processInteractionStyle == ProcessInteractionStyle::Immediate)
>             ResourceLoadObserver::shared().logUserInteractionWithReducedTimeResolution(document->topDocument());
>         document->topDocument().setUserDidInteractWithPage(true);
>+        if (auto* frame = document->frame())
>+            frame->setHasHadUserInteraction();
>     }
> }
> 
>diff --git a/Source/WebCore/page/Frame.h b/Source/WebCore/page/Frame.h
>index 9faf2437f5721e96a2582127e2eae2643db9b71e..dd238327fba3e146a99979b4007c76a7242d3ae4 100644
>--- a/Source/WebCore/page/Frame.h
>+++ b/Source/WebCore/page/Frame.h
>@@ -172,6 +172,9 @@ public:
> 
>     bool documentIsBeingReplaced() const { return m_documentIsBeingReplaced; }
> 
>+    void setHasHadUserInteraction() { m_hasHadUserInteraction = true; }
>+    bool hasHadUserInteraction() const { return m_hasHadUserInteraction; }
>+
> // ======== All public functions below this point are candidates to move out of Frame into another class. ========
> 
>     WEBCORE_EXPORT void injectUserScripts(UserScriptInjectionTime);
>@@ -346,6 +349,7 @@ private:
> 
>     int m_activeDOMObjectsAndAnimationsSuspendedCount;
>     bool m_documentIsBeingReplaced { false };
>+    bool m_hasHadUserInteraction { false };
>     unsigned m_navigationDisableCount { 0 };
>     unsigned m_selfOnlyRefCount { 0 };
> 
>diff --git a/Source/WebKit/Shared/FrameInfoData.cpp b/Source/WebKit/Shared/FrameInfoData.cpp
>index 9442374565816f4e88040ff084e0c1a3be531ff4..8bbe1ef821460e3d7ba26c18abd3d4dc1273aecc 100644
>--- a/Source/WebKit/Shared/FrameInfoData.cpp
>+++ b/Source/WebKit/Shared/FrameInfoData.cpp
>@@ -36,6 +36,7 @@ void FrameInfoData::encode(IPC::Encoder& encoder) const
>     encoder << request;
>     encoder << securityOrigin;
>     encoder << frameID;
>+    encoder << hasHadUserInteraction;
> }
> 
> bool FrameInfoData::decode(IPC::Decoder& decoder, FrameInfoData& result)
>@@ -51,6 +52,8 @@ bool FrameInfoData::decode(IPC::Decoder& decoder, FrameInfoData& result)
>     result.securityOrigin = WTFMove(*securityOrigin);
>     if (!decoder.decode(result.frameID))
>         return false;
>+    if (!decoder.decode(result.hasHadUserInteraction))
>+        return false;
> 
>     return true;
> }
>diff --git a/Source/WebKit/Shared/FrameInfoData.h b/Source/WebKit/Shared/FrameInfoData.h
>index a8283686f16ab3cd35249ce705d3f2b8e7bb3c66..3d0ed9d74223159f83bdd7042a02bf3835a895e4 100644
>--- a/Source/WebKit/Shared/FrameInfoData.h
>+++ b/Source/WebKit/Shared/FrameInfoData.h
>@@ -43,6 +43,7 @@ struct FrameInfoData {
>     WebCore::ResourceRequest request;
>     WebCore::SecurityOriginData securityOrigin;
>     uint64_t frameID { 0 };
>+    bool hasHadUserInteraction { false };
> };
> 
> }
>diff --git a/Source/WebKit/UIProcess/API/APIFrameInfo.cpp b/Source/WebKit/UIProcess/API/APIFrameInfo.cpp
>index 96b13d1785445c39733b2f048a60f2021508c8c8..91429257874a321999edce5500abb0b62c696b1e 100644
>--- a/Source/WebKit/UIProcess/API/APIFrameInfo.cpp
>+++ b/Source/WebKit/UIProcess/API/APIFrameInfo.cpp
>@@ -53,6 +53,7 @@ Ref<FrameInfo> FrameInfo::create(const WebKit::WebFrameProxy& frame, const WebCo
> 
> FrameInfo::FrameInfo(const WebKit::FrameInfoData& frameInfoData, WebKit::WebPageProxy* page)
>     : m_isMainFrame { frameInfoData.isMainFrame }
>+    , m_hasHadUserInteraction { frameInfoData.hasHadUserInteraction }
>     , m_request { frameInfoData.request }
>     , m_securityOrigin { SecurityOrigin::create(frameInfoData.securityOrigin.securityOrigin()) }
>     , m_handle { API::FrameHandle::create(frameInfoData.frameID) }
>diff --git a/Source/WebKit/UIProcess/API/APIFrameInfo.h b/Source/WebKit/UIProcess/API/APIFrameInfo.h
>index 9793dec82c68da2d25b52ef56fefaa9ac9f2a239..191981b7d8616ea13bad966e5f25059f13ab4e75 100644
>--- a/Source/WebKit/UIProcess/API/APIFrameInfo.h
>+++ b/Source/WebKit/UIProcess/API/APIFrameInfo.h
>@@ -50,6 +50,7 @@ public:
>     virtual ~FrameInfo();
> 
>     bool isMainFrame() const { return m_isMainFrame; }
>+    bool hasHadUserInteraction() const { return m_hasHadUserInteraction; }
>     const WebCore::ResourceRequest& request() const { return m_request; }
>     SecurityOrigin& securityOrigin() { return m_securityOrigin.get(); }
>     API::FrameHandle& handle() { return m_handle.get(); }
>@@ -61,6 +62,7 @@ private:
>     FrameInfo(const WebKit::FrameInfoData&, WebKit::WebPageProxy*);
> 
>     bool m_isMainFrame;
>+    bool m_hasHadUserInteraction { false };
>     WebCore::ResourceRequest m_request;
>     Ref<SecurityOrigin> m_securityOrigin;
>     Ref<FrameHandle> m_handle;
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKFrameInfo.mm b/Source/WebKit/UIProcess/API/Cocoa/WKFrameInfo.mm
>index 3d1a6de873e28d3fc81ed0dc506280e8157cbfce..44ed4db41904f51bffcb9838ff692f0233381b35 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKFrameInfo.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKFrameInfo.mm
>@@ -89,6 +89,11 @@ - (_WKFrameHandle *)_handle
>     return wrapper(_frameInfo->handle());
> }
> 
>+- (BOOL) _hasHadUserInteraction
>+{
>+    return _frameInfo->hasHadUserInteraction();
>+}
>+
> @end
> #endif
> 
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKFrameInfoPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKFrameInfoPrivate.h
>index f262c6726bb37e5bc07f2eb89bc27959c83f8bfb..262d632f40836fddadaf9b25e74b3dd6790457dc 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKFrameInfoPrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKFrameInfoPrivate.h
>@@ -32,6 +32,7 @@
> @interface WKFrameInfo (WKPrivate)
> 
> @property (nonatomic, readonly, strong) _WKFrameHandle *_handle WK_API_AVAILABLE(macosx(10.12), ios(10.0));
>+@property (nonatomic, readonly) BOOL _hasHadUserInteraction WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> @end
> 
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>index aacb5b9ce61b81657146a4bca653834ba0d400b0..650e21993c745c049b6e8c7ad21c62c67eabfd3e 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
>@@ -851,8 +851,13 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat
>     originatingFrameInfoData.isMainFrame = navigationAction.initiatedByMainFrame() == InitiatedByMainFrame::Yes;
>     originatingFrameInfoData.request = ResourceRequest { requester.url() };
>     originatingFrameInfoData.securityOrigin = requester.securityOrigin().data();
>-    if (requester.frameID() && WebProcess::singleton().webFrame(requester.frameID()))
>-        originatingFrameInfoData.frameID = requester.frameID();
>+    if (requester.frameID()) {
>+        if (auto* originatingFrame = WebProcess::singleton().webFrame(requester.frameID())) {
>+            originatingFrameInfoData.frameID = requester.frameID();
>+            if (auto* coreOriginatingFrame = originatingFrame->coreFrame())
>+                originatingFrameInfoData.hasHadUserInteraction = coreOriginatingFrame->hasHadUserInteraction();
>+        }
>+    }
> 
>     uint64_t originatingPageID = requester.pageID() && WebProcess::singleton().webPage(requester.pageID()) ? requester.pageID() : 0;
> 
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 15a17226450cd8b3e872b12282162683e1c6fea2..1b1c2c35a13aa0b3e6b89272c7eb8b8111d5082f 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-01-04  Chris Dumez  <cdumez@apple.com>
>+
>+        Add WKFrameInfo._hasHadUserInteraction SPI
>+        https://bugs.webkit.org/show_bug.cgi?id=193151
>+        <rdar://problem/36074736>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm:
>+        (TEST):
>+
> 2019-01-04  Chris Dumez  <cdumez@apple.com>
> 
>         Crash under WebProcessPool::addSuspendedPage()
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm
>index 19620446a85bdb3c4bc549d9b461a4929e6da146..0e84d56527b8eff0074b6c983e0935152656ee25 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/DecidePolicyForNavigationAction.mm
>@@ -29,8 +29,10 @@
> 
> #import "PlatformUtilities.h"
> #import "TestProtocol.h"
>+#import <WebKit/WKFrameInfoPrivate.h>
> #import <WebKit/WKNavigationActionPrivate.h>
> #import <WebKit/WKProcessPoolPrivate.h>
>+#import <WebKit/WKWebViewPrivate.h>
> #import <WebKit/_WKProcessPoolConfiguration.h>
> #import <wtf/RetainPtr.h>
> #import <wtf/mac/AppKitCompatibilityDeclarations.h>
>@@ -595,6 +597,37 @@ TEST(WebKit, DecidePolicyForNavigationActionForPOSTFormSubmissionThatRedirectsTo
>     action = nullptr;
> }
> 
>+TEST(WebKit, DecidePolicyForNavigationActionSourceFrameHasHadUserInteraction)
>+{
>+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
>+
>+    auto controller = adoptNS([[DecidePolicyForNavigationActionController alloc] init]);
>+    [webView setNavigationDelegate:controller.get()];
>+
>+    finishedNavigation = false;
>+    [webView loadHTMLString:@"<iframe srcdoc='FOO' name='subframe'></iframe><a id='testLink' href='about:blank' target='subframe'>Link</a>" baseURL:[NSURL URLWithString:@"http://webkit.org"]];
>+    TestWebKitAPI::Util::run(&finishedNavigation);
>+    finishedNavigation = false;
>+
>+    decidedPolicy = false;
>+    [webView _evaluateJavaScriptWithoutUserGesture:@"testLink.click()" completionHandler:nil];
>+    TestWebKitAPI::Util::run(&decidedPolicy);
>+    EXPECT_FALSE([action sourceFrame] == [action targetFrame]);
>+    EXPECT_FALSE([[action sourceFrame] _hasHadUserInteraction]);
>+
>+    decidedPolicy = false;
>+    [webView evaluateJavaScript:@"testLink.click()" completionHandler:nil];
>+    TestWebKitAPI::Util::run(&decidedPolicy);
>+    EXPECT_FALSE([action sourceFrame] == [action targetFrame]);
>+    EXPECT_TRUE([[action sourceFrame] _hasHadUserInteraction]);
>+
>+    decidedPolicy = false;
>+    [webView _evaluateJavaScriptWithoutUserGesture:@"testLink.click()" completionHandler:nil];
>+    TestWebKitAPI::Util::run(&decidedPolicy);
>+    EXPECT_FALSE([action sourceFrame] == [action targetFrame]);
>+    EXPECT_TRUE([[action sourceFrame] _hasHadUserInteraction]);
>+}
>+
> static size_t calls;
> static bool done;
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193151: 358346