Adopt RegistrableDomain in the Storage Access API prompt code paths
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Mar 2019 16:41:15 +0000 (16:41 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Mar 2019 16:41:15 +0000 (16:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195957
<rdar://problem/49048028>

Reviewed by Chris Dumez.

While making the code changes in Bug 195866 we noticed that strings were being
used to pass the origins for the API calls. We should adopt the RegistrableDomain
class to improve type safety and avoid introducing bugs in the future.

* UIProcess/API/APIUIClient.h:
(API::UIClient::requestStorageAccessConfirm):
* UIProcess/API/C/WKPage.cpp:
(WKPageSetPageUIClient):
* UIProcess/Cocoa/UIDelegate.h:
* UIProcess/Cocoa/UIDelegate.mm:
(WebKit::UIDelegate::UIClient::requestStorageAccessConfirm):
* UIProcess/Cocoa/WKStorageAccessAlert.h:
* UIProcess/Cocoa/WKStorageAccessAlert.mm:
(WebKit::presentStorageAccessAlert):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::requestStorageAccessConfirm):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APIUIClient.h
Source/WebKit/UIProcess/API/C/WKPage.cpp
Source/WebKit/UIProcess/Cocoa/UIDelegate.h
Source/WebKit/UIProcess/Cocoa/UIDelegate.mm
Source/WebKit/UIProcess/Cocoa/WKStorageAccessAlert.h
Source/WebKit/UIProcess/Cocoa/WKStorageAccessAlert.mm
Source/WebKit/UIProcess/WebPageProxy.cpp

index bcbaa8b..43609fe 100644 (file)
@@ -1,3 +1,28 @@
+2019-03-20  Brent Fulgham  <bfulgham@apple.com>
+
+        Adopt RegistrableDomain in the Storage Access API prompt code paths
+        https://bugs.webkit.org/show_bug.cgi?id=195957
+        <rdar://problem/49048028>
+
+        Reviewed by Chris Dumez.
+
+        While making the code changes in Bug 195866 we noticed that strings were being
+        used to pass the origins for the API calls. We should adopt the RegistrableDomain
+        class to improve type safety and avoid introducing bugs in the future.
+
+        * UIProcess/API/APIUIClient.h:
+        (API::UIClient::requestStorageAccessConfirm):
+        * UIProcess/API/C/WKPage.cpp:
+        (WKPageSetPageUIClient):
+        * UIProcess/Cocoa/UIDelegate.h:
+        * UIProcess/Cocoa/UIDelegate.mm:
+        (WebKit::UIDelegate::UIClient::requestStorageAccessConfirm):
+        * UIProcess/Cocoa/WKStorageAccessAlert.h:
+        * UIProcess/Cocoa/WKStorageAccessAlert.mm:
+        (WebKit::presentStorageAccessAlert):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::requestStorageAccessConfirm):
+
 2019-03-20  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, fix typo in comment added in r243156.
index c2121ef..d9dd0db 100644 (file)
@@ -39,6 +39,7 @@ OBJC_CLASS UIViewController;
 #endif
 
 namespace WebCore {
+class RegistrableDomain;
 class ResourceRequest;
 struct FontAttributes;
 struct SecurityOriginData;
@@ -132,7 +133,7 @@ public:
     virtual void decidePolicyForUserMediaPermissionRequest(WebKit::WebPageProxy&, WebKit::WebFrameProxy&, SecurityOrigin&, SecurityOrigin&, WebKit::UserMediaPermissionRequestProxy& request) { request.deny(); }
     virtual void checkUserMediaPermissionForOrigin(WebKit::WebPageProxy&, WebKit::WebFrameProxy&, SecurityOrigin&, SecurityOrigin&, WebKit::UserMediaPermissionCheckProxy& request) { request.deny(); }
     virtual void decidePolicyForNotificationPermissionRequest(WebKit::WebPageProxy&, SecurityOrigin&, Function<void(bool)>&& completionHandler) { completionHandler(false); }
-    virtual void requestStorageAccessConfirm(WebKit::WebPageProxy&, WebKit::WebFrameProxy*, const WTF::String& requestingDomain, const WTF::String& currentDomain, CompletionHandler<void(bool)>&& completionHandler) { completionHandler(true); }
+    virtual void requestStorageAccessConfirm(WebKit::WebPageProxy&, WebKit::WebFrameProxy*, const WebCore::RegistrableDomain& requestingDomain, const WebCore::RegistrableDomain& currentDomain, CompletionHandler<void(bool)>&& completionHandler) { completionHandler(true); }
 
     // Printing.
     virtual float headerHeight(WebKit::WebPageProxy&, WebKit::WebFrameProxy&) { return 0; }
index d9cb3c6..f8f39ab 100644 (file)
@@ -1888,7 +1888,7 @@ void WKPageSetPageUIClient(WKPageRef pageRef, const WKPageUIClientBase* wkClient
             m_client.decidePolicyForNotificationPermissionRequest(toAPI(&page), toAPI(&origin), toAPI(NotificationPermissionRequest::create(WTFMove(completionHandler)).ptr()), m_client.base.clientInfo);
         }
 
-        void requestStorageAccessConfirm(WebPageProxy& page, WebFrameProxy* frame, const WTF::String& requestingDomain, const WTF::String& currentDomain, CompletionHandler<void(bool)>&& completionHandler) final
+        void requestStorageAccessConfirm(WebPageProxy& page, WebFrameProxy* frame, const WebCore::RegistrableDomain& requestingDomain, const WebCore::RegistrableDomain& currentDomain, CompletionHandler<void(bool)>&& completionHandler) final
         {
             if (!m_client.requestStorageAccessConfirm) {
                 completionHandler(true);
@@ -1896,7 +1896,7 @@ void WKPageSetPageUIClient(WKPageRef pageRef, const WKPageUIClientBase* wkClient
             }
 
             auto listener = RequestStorageAccessConfirmResultListener::create(WTFMove(completionHandler));
-            m_client.requestStorageAccessConfirm(toAPI(&page), toAPI(frame), toAPI(requestingDomain.impl()), toAPI(currentDomain.impl()), toAPI(listener.ptr()), m_client.base.clientInfo);
+            m_client.requestStorageAccessConfirm(toAPI(&page), toAPI(frame), toAPI(requestingDomain.string().impl()), toAPI(currentDomain.string().impl()), toAPI(listener.ptr()), m_client.base.clientInfo);
         }
 
 #if ENABLE(DEVICE_ORIENTATION)
index 3c23f8b..bd20a81 100644 (file)
@@ -41,6 +41,10 @@ class FrameInfo;
 class SecurityOrigin;
 }
 
+namespace WebCore {
+class RegistrableDomain;
+}
+
 namespace WebKit {
 
 class UIDelegate {
@@ -88,7 +92,7 @@ private:
         void runJavaScriptConfirm(WebPageProxy*, const WTF::String&, WebFrameProxy*, const WebCore::SecurityOriginData&, Function<void(bool)>&& completionHandler) final;
         void runJavaScriptPrompt(WebPageProxy*, const WTF::String&, const WTF::String&, WebFrameProxy*, const WebCore::SecurityOriginData&, Function<void(const WTF::String&)>&&) final;
         void presentStorageAccessConfirmDialog(const WTF::String& requestingDomain, const WTF::String& currentDomain, CompletionHandler<void(bool)>&&);
-        void requestStorageAccessConfirm(WebPageProxy&, WebFrameProxy*, const WTF::String& requestingDomain, const WTF::String& currentDomain, CompletionHandler<void(bool)>&&) final;
+        void requestStorageAccessConfirm(WebPageProxy&, WebFrameProxy*, const WebCore::RegistrableDomain& requestingDomain, const WebCore::RegistrableDomain& currentDomain, CompletionHandler<void(bool)>&&) final;
         void decidePolicyForGeolocationPermissionRequest(WebPageProxy&, WebFrameProxy&, API::SecurityOrigin&, Function<void(bool)>&) final;
         bool canRunBeforeUnloadConfirmPanel() const final;
         void runBeforeUnloadConfirmPanel(WebPageProxy*, const WTF::String&, WebFrameProxy*, const WebCore::SecurityOriginData&, Function<void(bool)>&& completionHandler) final;
@@ -163,7 +167,7 @@ private:
         bool webViewRunJavaScriptAlertPanelWithMessageInitiatedByFrameCompletionHandler : 1;
         bool webViewRunJavaScriptConfirmPanelWithMessageInitiatedByFrameCompletionHandler : 1;
         bool webViewRunJavaScriptTextInputPanelWithPromptDefaultTextInitiatedByFrameCompletionHandler : 1;
-        bool webViewRequestStorageAccessPanelForTopPrivatelyControlledDomainUnderFirstPartyTopPrivatelyControlledDomainCompletionHandler : 1;
+        bool webViewRequestStorageAccessPanelUnderFirstPartyCompletionHandler : 1;
         bool webViewRunBeforeUnloadConfirmPanelWithMessageInitiatedByFrameCompletionHandler : 1;
         bool webViewRequestGeolocationPermissionForFrameDecisionHandler : 1;
         bool webViewDidResignInputElementStrongPasswordAppearanceWithUserInfo : 1;
index c623893..a4ad877 100644 (file)
@@ -104,7 +104,7 @@ void UIDelegate::setDelegate(id <WKUIDelegate> delegate)
     m_delegateMethods.webViewRunJavaScriptAlertPanelWithMessageInitiatedByFrameCompletionHandler = [delegate respondsToSelector:@selector(webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:)];
     m_delegateMethods.webViewRunJavaScriptConfirmPanelWithMessageInitiatedByFrameCompletionHandler = [delegate respondsToSelector:@selector(webView:runJavaScriptConfirmPanelWithMessage:initiatedByFrame:completionHandler:)];
     m_delegateMethods.webViewRunJavaScriptTextInputPanelWithPromptDefaultTextInitiatedByFrameCompletionHandler = [delegate respondsToSelector:@selector(webView:runJavaScriptTextInputPanelWithPrompt:defaultText:initiatedByFrame:completionHandler:)];
-    m_delegateMethods.webViewRequestStorageAccessPanelForTopPrivatelyControlledDomainUnderFirstPartyTopPrivatelyControlledDomainCompletionHandler = [delegate respondsToSelector:@selector(_webView:requestStorageAccessPanelForDomain:underCurrentDomain:completionHandler:)];
+    m_delegateMethods.webViewRequestStorageAccessPanelUnderFirstPartyCompletionHandler = [delegate respondsToSelector:@selector(_webView:requestStorageAccessPanelForDomain:underCurrentDomain:completionHandler:)];
     m_delegateMethods.webViewRunBeforeUnloadConfirmPanelWithMessageInitiatedByFrameCompletionHandler = [delegate respondsToSelector:@selector(_webView:runBeforeUnloadConfirmPanelWithMessage:initiatedByFrame:completionHandler:)];
     m_delegateMethods.webViewRequestGeolocationPermissionForFrameDecisionHandler = [delegate respondsToSelector:@selector(_webView:requestGeolocationPermissionForFrame:decisionHandler:)];
     m_delegateMethods.webViewDidResignInputElementStrongPasswordAppearanceWithUserInfo = [delegate respondsToSelector:@selector(_webView:didResignInputElementStrongPasswordAppearanceWithUserInfo:)];
@@ -337,7 +337,7 @@ void UIDelegate::UIClient::runJavaScriptPrompt(WebPageProxy*, const WTF::String&
     }).get()];
 }
 
-void UIDelegate::UIClient::requestStorageAccessConfirm(WebPageProxy&, WebFrameProxy*, const WTF::String& requestingDomain, const WTF::String& currentDomain, CompletionHandler<void(bool)>&& completionHandler)
+void UIDelegate::UIClient::requestStorageAccessConfirm(WebPageProxy&, WebFrameProxy*, const WebCore::RegistrableDomain& requestingDomain, const WebCore::RegistrableDomain& currentDomain, CompletionHandler<void(bool)>&& completionHandler)
 {
     auto delegate = m_uiDelegate.m_delegate.get();
     if (!delegate) {
@@ -345,7 +345,7 @@ void UIDelegate::UIClient::requestStorageAccessConfirm(WebPageProxy&, WebFramePr
         return;
     }
     
-    if (!m_uiDelegate.m_delegateMethods.webViewRequestStorageAccessPanelForTopPrivatelyControlledDomainUnderFirstPartyTopPrivatelyControlledDomainCompletionHandler) {
+    if (!m_uiDelegate.m_delegateMethods.webViewRequestStorageAccessPanelUnderFirstPartyCompletionHandler) {
 #if !PLATFORM(WATCHOS) && !PLATFORM(APPLETV)
         presentStorageAccessAlert(m_uiDelegate.m_webView, requestingDomain, currentDomain, WTFMove(completionHandler));
 #endif
@@ -353,7 +353,7 @@ void UIDelegate::UIClient::requestStorageAccessConfirm(WebPageProxy&, WebFramePr
     }
 
     auto checker = CompletionHandlerCallChecker::create(delegate.get(), @selector(_webView:requestStorageAccessPanelForDomain:underCurrentDomain:completionHandler:));
-    [(id <WKUIDelegatePrivate>)delegate _webView:m_uiDelegate.m_webView requestStorageAccessPanelForDomain:requestingDomain underCurrentDomain:currentDomain completionHandler:makeBlockPtr([completionHandler = WTFMove(completionHandler), checker = WTFMove(checker)] (BOOL result) mutable {
+    [(id <WKUIDelegatePrivate>)delegate _webView:m_uiDelegate.m_webView requestStorageAccessPanelForDomain:requestingDomain.string() underCurrentDomain:currentDomain.string() completionHandler:makeBlockPtr([completionHandler = WTFMove(completionHandler), checker = WTFMove(checker)] (BOOL result) mutable {
         if (checker->completionHandlerHasBeenCalled())
             return;
         completionHandler(result);
index 12e590d..cc1931b 100644 (file)
@@ -35,9 +35,13 @@ namespace WTF {
 class String;
 }
 
+namespace WebCore {
+class RegistrableDomain;
+}
+
 namespace WebKit {
 
-void presentStorageAccessAlert(WKWebView *, const WTF::String& requestingDomain, const WTF::String& currentDomain, CompletionHandler<void(bool)>&&);
+void presentStorageAccessAlert(WKWebView *, const WebCore::RegistrableDomain& requestingDomain, const WebCore::RegistrableDomain& currentDomain, CompletionHandler<void(bool)>&&);
 
 }
 
index 69e6ab0..c72eeaf 100644 (file)
 
 #import "WKWebViewInternal.h"
 #import <WebCore/LocalizedStrings.h>
+#import <WebCore/RegistrableDomain.h>
 #import <wtf/BlockPtr.h>
 
 namespace WebKit {
 
-void presentStorageAccessAlert(WKWebView *webView, const WTF::String& requestingDomain, const WTF::String& currentDomain, CompletionHandler<void(bool)>&& completionHandler)
+void presentStorageAccessAlert(WKWebView *webView, const WebCore::RegistrableDomain& requesting, const WebCore::RegistrableDomain& current, CompletionHandler<void(bool)>&& completionHandler)
 {
     auto completionBlock = makeBlockPtr([completionHandler = WTFMove(completionHandler)](bool shouldAllow) mutable {
         completionHandler(shouldAllow);
     });
 
+    auto requestingDomain = requesting.string().createCFString();
+    auto currentDomain = current.string().createCFString();
+
 #if PLATFORM(MAC)
-    NSString *alertTitle = [NSString stringWithFormat:WEB_UI_NSSTRING(@"Do you want to allow \"%@\" to use cookies and website data while browsing \"%@\"?", @"Message for requesting cross-site cookie and website data access."), requestingDomain.createCFString().get(), currentDomain.createCFString().get()];
+    NSString *alertTitle = [NSString stringWithFormat:WEB_UI_NSSTRING(@"Do you want to allow \"%@\" to use cookies and website data while browsing \"%@\"?", @"Message for requesting cross-site cookie and website data access."), requestingDomain.get(), currentDomain.get()];
 #else
-    NSString *alertTitle = [NSString stringWithFormat:WEB_UI_NSSTRING(@"Allow \"%@\" to use cookies and website data while browsing \"%@\"?", @"Message for requesting cross-site cookie and website data access."), requestingDomain.createCFString().get(), currentDomain.createCFString().get()];
+    NSString *alertTitle = [NSString stringWithFormat:WEB_UI_NSSTRING(@"Allow \"%@\" to use cookies and website data while browsing \"%@\"?", @"Message for requesting cross-site cookie and website data access."), requestingDomain.get(), currentDomain.get()];
 #endif
 
-    NSString *informativeText = [NSString stringWithFormat:WEB_UI_NSSTRING(@"This will allow \"%@\" to track your activity.", @"Informative text for requesting cross-site cookie and website data access."), requestingDomain.createCFString().get()];
+    NSString *informativeText = [NSString stringWithFormat:WEB_UI_NSSTRING(@"This will allow \"%@\" to track your activity.", @"Informative text for requesting cross-site cookie and website data access."), requestingDomain.get()];
     NSString *allowButtonString = WEB_UI_STRING_KEY(@"Allow", "Allow (cross-site cookie and website data access)", @"Button title in Storage Access API prompt");
     NSString *doNotAllowButtonString = WEB_UI_STRING_KEY(@"Don't Allow", "Don't Allow (cross-site cookie and website data access)", @"Button title in Storage Access API prompt");
 
index 3aecc04..eb02a60 100644 (file)
@@ -8459,7 +8459,7 @@ void WebPageProxy::loadSynchronousURLSchemeTask(URLSchemeTaskParameters&& parame
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
 void WebPageProxy::requestStorageAccessConfirm(const RegistrableDomain& subFrameDomain, const RegistrableDomain& topFrameDomain, uint64_t frameID, CompletionHandler<void(bool)>&& completionHandler)
 {
-    m_uiClient->requestStorageAccessConfirm(*this, m_process->webFrame(frameID), subFrameDomain.string(), topFrameDomain.string(), WTFMove(completionHandler));
+    m_uiClient->requestStorageAccessConfirm(*this, m_process->webFrame(frameID), subFrameDomain, topFrameDomain, WTFMove(completionHandler));
 }
 #endif