Attachment 343425 Details for Bug 186963 – Patch

[patch] Patch

bug-186963-20180622234428.patch (text/plain), 8.06 KB, created by Tim Horton on 2018-06-22 23:44:29 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Horton

Created: 2018-06-22 23:44:29 PDT

Size: 8.06 KB

patch

obsolete

>Subversion Revision: 233108
>diff --git a/Source/WebCore/PAL/ChangeLog b/Source/WebCore/PAL/ChangeLog
>index 84b85a8e978d02afdd67ceb931662895a2a57363..17f5ad5d08a12c194ca7c8e538145a2dd6c55f43 100644
>--- a/Source/WebCore/PAL/ChangeLog
>+++ b/Source/WebCore/PAL/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-06-22  Tim Horton  <timothy_horton@apple.com>
>+
>+        Creating and loading content in a WKWebView triggers Main Thread Checker warnings under ServicesController
>+        https://bugs.webkit.org/show_bug.cgi?id=186963
>+        <rdar://problem/41393682>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * pal/spi/mac/NSSharingServiceSPI.h:
>+        Add the new SPI.
>+
> 2018-06-21  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed attempt to fix the build after r233052.
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 35818f0d8d34515b3cfbe5d6b80ee520c192336f..26b3d08bedb404b72800f25f93824eca553eb66d 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-06-22  Tim Horton  <timothy_horton@apple.com>
>+
>+        Creating and loading content in a WKWebView triggers Main Thread Checker warnings under ServicesController
>+        https://bugs.webkit.org/show_bug.cgi?id=186963
>+        <rdar://problem/41393682>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/mac/ServicesController.h:
>+        * UIProcess/mac/ServicesController.mm:
>+        (WebKit::hasCompatibleServicesForItems):
>+        (WebKit::ServicesController::refreshExistingServices):
>+        Adopt async ShareKit SPI that is actually thread-safe instead of the not-quite-safe
>+        synchronous API. Request all three sets of services immediately, and dispatch
>+        to the Web Content processes when all three have returned.
>+
> 2018-06-22  Sihui Liu  <sihui_liu@apple.com>
> 
>         REGRESSION (r231850): Cookie file cannot be read or written by network process
>diff --git a/Source/WebCore/PAL/pal/spi/mac/NSSharingServiceSPI.h b/Source/WebCore/PAL/pal/spi/mac/NSSharingServiceSPI.h
>index 4a172dae83dd9cea078e09ef7eb44e2975b99e88..4d3bb5c5f1f71987f84c91192821ae353b071a27 100644
>--- a/Source/WebCore/PAL/pal/spi/mac/NSSharingServiceSPI.h
>+++ b/Source/WebCore/PAL/pal/spi/mac/NSSharingServiceSPI.h
>@@ -45,8 +45,9 @@ typedef NS_OPTIONS(NSUInteger, NSSharingServiceMask) {
>     NSSharingServiceMaskAllTypes = 0xFFFF
> } NS_ENUM_AVAILABLE_MAC(10_10);
> 
>-@interface NSSharingService (Private)
>+@interface NSSharingService ()
> + (NSArray *)sharingServicesForItems:(NSArray *)items mask:(NSSharingServiceMask)maskForFiltering;
>++ (void)sharingServicesForItems:(NSArray *)items mask:(NSSharingServiceMask)maskForFiltering completion:(void(^)(NSArray *))completion;
> @property (readonly) NSSharingServiceType type;
> @property (readwrite, copy) NSString *name;
> @end
>diff --git a/Source/WebKit/UIProcess/mac/ServicesController.h b/Source/WebKit/UIProcess/mac/ServicesController.h
>index 67c60ee9ddbb76342732516bb2c1226d7bf46a30..df5ff9b99a2dc7810dcc11a065c057abf7e1d425 100644
>--- a/Source/WebKit/UIProcess/mac/ServicesController.h
>+++ b/Source/WebKit/UIProcess/mac/ServicesController.h
>@@ -52,9 +52,13 @@ private:
>     dispatch_queue_t m_refreshQueue;
>     std::atomic_bool m_hasPendingRefresh;
> 
>-    bool m_hasImageServices;
>-    bool m_hasSelectionServices;
>-    bool m_hasRichContentServices;
>+    std::atomic<bool> m_hasImageServices;
>+    std::atomic<bool> m_hasSelectionServices;
>+    std::atomic<bool> m_hasRichContentServices;
>+
>+    bool m_lastSentHasImageServices;
>+    bool m_lastSentHasSelectionServices;
>+    bool m_lastSentHasRichContentServices;
> 
>     RetainPtr<id> m_extensionWatcher;
>     RetainPtr<id> m_uiExtensionWatcher;
>diff --git a/Source/WebKit/UIProcess/mac/ServicesController.mm b/Source/WebKit/UIProcess/mac/ServicesController.mm
>index 9926429b427a1690acf2dba62ce5cc5356ea2c52..ded1b459e1dfdbb7fa2553bca139e30b051f8ace 100644
>--- a/Source/WebKit/UIProcess/mac/ServicesController.mm
>+++ b/Source/WebKit/UIProcess/mac/ServicesController.mm
>@@ -33,6 +33,7 @@
> #import <pal/spi/cocoa/NSExtensionSPI.h>
> #import <pal/spi/mac/NSSharingServicePickerSPI.h>
> #import <pal/spi/mac/NSSharingServiceSPI.h>
>+#import <wtf/BlockPtr.h>
> #import <wtf/NeverDestroyed.h>
> 
> namespace WebKit {
>@@ -65,9 +66,20 @@ ServicesController::ServicesController()
> #endif // __LP64__
> }
> 
>-static bool hasCompatibleServicesForItems(NSArray *items)
>+static void hasCompatibleServicesForItems(dispatch_group_t group, NSArray *items, WTF::Function<void(bool)>&& completionHandler)
> {
>-    return [NSSharingService sharingServicesForItems:items mask:NSSharingServiceMaskViewer | NSSharingServiceMaskEditor].count;
>+    NSSharingServiceMask servicesMask = NSSharingServiceMaskViewer | NSSharingServiceMaskEditor;
>+
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
>+    dispatch_group_enter(group);
>+    [NSSharingService sharingServicesForItems:items mask:servicesMask completion:BlockPtr<void(NSArray *)>::fromCallable([completionHandler = WTFMove(completionHandler), group](NSArray *services) {
>+        completionHandler(services.count);
>+        dispatch_group_leave(group);
>+    }).get()];
>+#else
>+    UNUSED_PARAM(group);
>+    completionHandler([NSSharingService sharingServicesForItems:items mask:servicesMask].count);
>+#endif
> }
> 
> void ServicesController::refreshExistingServices(bool refreshImmediately)
>@@ -79,11 +91,17 @@ void ServicesController::refreshExistingServices(bool refreshImmediately)
> 
>     auto refreshTime = dispatch_time(DISPATCH_TIME_NOW, refreshImmediately ? 0 : (int64_t)(1 * NSEC_PER_SEC));
>     dispatch_after(refreshTime, m_refreshQueue, ^{
>+        auto serviceLookupGroup = adoptOSObject(dispatch_group_create());
>+
>         static NeverDestroyed<NSImage *> image([[NSImage alloc] init]);
>-        bool hasImageServices = hasCompatibleServicesForItems(@[ image ]);
>+        hasCompatibleServicesForItems(serviceLookupGroup.get(), @[ image ], [this] (bool hasServices) {
>+            m_hasImageServices = hasServices;
>+        });
> 
>         static NeverDestroyed<NSAttributedString *> attributedString([[NSAttributedString alloc] initWithString:@"a"]);
>-        bool hasSelectionServices = hasCompatibleServicesForItems(@[ attributedString ]);
>+        hasCompatibleServicesForItems(serviceLookupGroup.get(), @[ attributedString ], [this] (bool hasServices) {
>+            m_hasSelectionServices = hasServices;
>+        });
> 
>         static NSAttributedString *attributedStringWithRichContent;
>         if (!attributedStringWithRichContent) {
>@@ -97,14 +115,16 @@ void ServicesController::refreshExistingServices(bool refreshImmediately)
>             });
>         }
> 
>-        bool hasRichContentServices = hasCompatibleServicesForItems(@[ attributedStringWithRichContent ]);
>-        
>-        dispatch_async(dispatch_get_main_queue(), ^{
>-            bool availableServicesChanged = (hasImageServices != m_hasImageServices) || (hasSelectionServices != m_hasSelectionServices) || (hasRichContentServices != m_hasRichContentServices);
>+        hasCompatibleServicesForItems(serviceLookupGroup.get(), @[ attributedStringWithRichContent ], [this] (bool hasServices) {
>+            m_hasRichContentServices = hasServices;
>+        });
>+
>+        dispatch_group_notify(serviceLookupGroup.get(), dispatch_get_main_queue(), BlockPtr<void(void)>::fromCallable([this] {
>+            bool availableServicesChanged = (m_lastSentHasImageServices != m_hasImageServices) || (m_lastSentHasSelectionServices != m_hasSelectionServices) || (m_lastSentHasRichContentServices != m_hasRichContentServices);
> 
>-            m_hasSelectionServices = hasSelectionServices;
>-            m_hasImageServices = hasImageServices;
>-            m_hasRichContentServices = hasRichContentServices;
>+            m_lastSentHasSelectionServices = m_hasSelectionServices;
>+            m_lastSentHasImageServices = m_hasImageServices;
>+            m_lastSentHasRichContentServices = m_hasRichContentServices;
> 
>             if (availableServicesChanged) {
>                 for (auto& processPool : WebProcessPool::allProcessPools())
>@@ -112,7 +132,7 @@ void ServicesController::refreshExistingServices(bool refreshImmediately)
>             }
> 
>             m_hasPendingRefresh = false;
>-        });
>+        }).get());
>     });
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 186963: 343424 | 343425