Attachment 347219 Details for Bug 188621 – Supresses the release assert

[patch] Supresses the release assert

bug-188621-20180815160955.patch (text/plain), 10.14 KB, created by Ryosuke Niwa on 2018-08-15 16:09:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-08-15 16:09:55 PDT

Size: 10.14 KB

patch

obsolete

>Subversion Revision: 234870
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e35e1d43d085ac3c176567d6269b2791ac93e8ff..1502f432ef5603ba47bf852ebc2f219278f96941 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-08-15  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Can't share an app on AppStore to WeChat due to a release assert
>+        https://bugs.webkit.org/show_bug.cgi?id=188621
>+        <rdar://problem/43343976>
>+
>+        Reviewed by Geoffrey Garen.
>+
>+        Disable the thread safety check when the app is not linked on or after iOS 12 since this release assert
>+        is getting hit by third party applications on iOS in UI process.
>+
>+        * platform/Timer.cpp:
>+        (WebCore::shouldSuppressThreadSafetyCheck): Added a SDK check.
>+
> 2018-08-14  Ansh Shukla  <ansh_shukla@apple.com>
> 
>         NSURLAuthenticationMethodOAuth challenges are surfaced to clients in -didReceiveAuthenticationChallenge as NSURLAuthenticationMethodDefault
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index a8ff3d658057db13c11bcbcece6af9d7bbb30e62..3d7122da73f98ddcc0fd3134ab4ad1a11a687d90 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-08-15  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        Can't share an app on AppStore to WeChat due to a release assert
>+        https://bugs.webkit.org/show_bug.cgi?id=188621
>+        <rdar://problem/43343976>
>+
>+        Reviewed by Geoffrey Garen.
>+
>+        Disable the thread safety check when the app is not linked on or after iOS 12 since this release assert
>+        is getting hit by third party applications on iOS in UI process.
>+
>+        * UIProcess/Cocoa/VersionChecks.h:
>+        (WebKit::SDKVersion::FirstWithMainThreadReleaseAssertionInWebPageProxy): Added. It's iOS 12 or macOS 10.14 Mojave.
>+        * UIProcess/WebProcessProxy.cpp:
>+        (WebKit::isMainThreadOrCheckDisabled): Added. Returns true whether when we're in the main thread or if the app
>+        is not linked on or after iOS 12 or macOS 10.14 Mojave.
>+        (WebKit::globalPageMap):
>+        (WebKit::m_isInPrewarmedPool):
>+        (WebKit::WebProcessProxy::~WebProcessProxy):
>+        (WebKit::WebProcessProxy::shutDown):
>+        (WebKit::WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores):
>+        (WebKit::WebProcessProxy::topPrivatelyControlledDomainsWithWebsiteData):
>+        (WebKit::WebProcessProxy::didFinishLaunching):
>+
> 2018-08-14  Ansh Shukla  <ansh_shukla@apple.com>
> 
>         NSURLAuthenticationMethodOAuth challenges are surfaced to clients in -didReceiveAuthenticationChallenge as NSURLAuthenticationMethodDefault
>diff --git a/Source/WebCore/platform/Timer.cpp b/Source/WebCore/platform/Timer.cpp
>index 239e1c9e0aabb664c8d79ad3d29471f36aaf2d43..3b59825ab6b7b127eb2747945950dae7d5f6a5bb 100644
>--- a/Source/WebCore/platform/Timer.cpp
>+++ b/Source/WebCore/platform/Timer.cpp
>@@ -37,7 +37,7 @@
> #include <wtf/MainThread.h>
> #include <wtf/Vector.h>
> 
>-#if USE(WEB_THREAD) || PLATFORM(MAC)
>+#if PLATFORM(IOS) || PLATFORM(MAC)
> #include <wtf/spi/darwin/dyldSPI.h>
> #endif
> 
>@@ -191,8 +191,8 @@ inline bool TimerHeapLessThanFunction::operator()(const TimerBase* a, const Time
> 
> static bool shouldSuppressThreadSafetyCheck()
> {
>-#if USE(WEB_THREAD)
>-    return WebThreadIsEnabled();
>+#if PLATFORM(IOS)
>+    return WebThreadIsEnabled() || applicationSDKVersion() < DYLD_IOS_VERSION_12_0;
> #elif PLATFORM(MAC)
>     return !isInWebProcess() && applicationSDKVersion() < DYLD_MACOSX_VERSION_10_14;
> #else
>diff --git a/Source/WebKit/UIProcess/Cocoa/VersionChecks.h b/Source/WebKit/UIProcess/Cocoa/VersionChecks.h
>index 15035b3c010f278d78f756ee33eed7beaa4913a5..669e7d1a152b446b2d9496f2c8bb40afc3182709 100644
>--- a/Source/WebKit/UIProcess/Cocoa/VersionChecks.h
>+++ b/Source/WebKit/UIProcess/Cocoa/VersionChecks.h
>@@ -40,11 +40,13 @@ enum class SDKVersion : uint32_t {
>     FirstThatDefaultsToPassiveTouchListenersOnDocument = DYLD_IOS_VERSION_11_3,
>     FirstWhereScrollViewContentInsetsAreNotObscuringInsets = DYLD_IOS_VERSION_12_0,
>     FirstWhereUIScrollViewDoesNotApplyKeyboardInsetsUnconditionally = DYLD_IOS_VERSION_12_0,
>+    FirstWithMainThreadReleaseAssertionInWebPageProxy = DYLD_IOS_VERSION_12_0,
> #elif PLATFORM(MAC)
>     FirstWithNetworkCache = DYLD_MACOSX_VERSION_10_11,
>     FirstWithExceptionsForDuplicateCompletionHandlerCalls = DYLD_MACOSX_VERSION_10_13,
>     FirstWithDropToNavigateDisallowedByDefault = DYLD_MACOSX_VERSION_10_13,
>     FirstWithExpiredOnlyReloadBehavior = DYLD_MACOSX_VERSION_10_13,
>+    FirstWithMainThreadReleaseAssertionInWebPageProxy = DYLD_MACOSX_VERSION_10_14,
> #endif
> };
> 
>diff --git a/Source/WebKit/UIProcess/WebProcessProxy.cpp b/Source/WebKit/UIProcess/WebProcessProxy.cpp
>index 1abee56f86ca5e058459e3f7a03f51c77cc95bc8..37deb64c3bab4bd598dccd1200f2c2eab1f67ace 100644
>--- a/Source/WebKit/UIProcess/WebProcessProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessProxy.cpp
>@@ -39,6 +39,7 @@
> #include "TextCheckerState.h"
> #include "UIMessagePortChannelProvider.h"
> #include "UserData.h"
>+#include "VersionChecks.h"
> #include "WebBackForwardListItem.h"
> #include "WebInspectorUtilities.h"
> #include "WebNavigationDataStore.h"
>@@ -80,9 +81,20 @@ using namespace WebCore;
> 
> namespace WebKit {
> 
>+static bool isMainThreadOrCheckDisabled()
>+{
>+#if PLATFORM(IOS)
>+    return LIKELY(RunLoop::isMain()) || !linkedOnOrAfter(SDKVersion::FirstWithMainThreadReleaseAssertionInWebPageProxy);
>+#elif PLATFORM(MAC)
>+    return LIKELY(RunLoop::isMain()) || !linkedOnOrAfter(SDKVersion::FirstWithMainThreadReleaseAssertionInWebPageProxy);
>+#else
>+    return RunLoop::isMain();
>+#endif
>+}
>+
> static HashMap<ProcessIdentifier, WebProcessProxy*>& allProcesses()
> {
>-    ASSERT(RunLoop::isMain());
>+    ASSERT(isMainThreadOrCheckDisabled());
>     static NeverDestroyed<HashMap<ProcessIdentifier, WebProcessProxy*>> map;
>     return map;
> }
>@@ -100,7 +112,7 @@ uint64_t WebProcessProxy::generatePageID()
> 
> static WebProcessProxy::WebPageProxyMap& globalPageMap()
> {
>-    ASSERT(RunLoop::isMain());
>+    ASSERT(isMainThreadOrCheckDisabled());
>     static NeverDestroyed<WebProcessProxy::WebPageProxyMap> pageMap;
>     return pageMap;
> }
>@@ -128,7 +140,7 @@ WebProcessProxy::WebProcessProxy(WebProcessPool& processPool, WebsiteDataStore&
> #endif
>     , m_isInPrewarmedPool(isInPrewarmedPool == IsInPrewarmedPool::Yes)
> {
>-    RELEASE_ASSERT(RunLoop::isMain());
>+    RELEASE_ASSERT(isMainThreadOrCheckDisabled());
> 
>     auto result = allProcesses().add(coreProcessIdentifier(), this);
>     ASSERT_UNUSED(result, result.isNewEntry);
>@@ -138,7 +150,7 @@ WebProcessProxy::WebProcessProxy(WebProcessPool& processPool, WebsiteDataStore&
> 
> WebProcessProxy::~WebProcessProxy()
> {
>-    RELEASE_ASSERT(RunLoop::isMain());
>+    RELEASE_ASSERT(isMainThreadOrCheckDisabled());
>     ASSERT(m_pageURLRetainCountMap.isEmpty());
> 
>     auto result = allProcesses().remove(coreProcessIdentifier());
>@@ -212,7 +224,7 @@ void WebProcessProxy::processWillShutDown(IPC::Connection& connection)
> 
> void WebProcessProxy::shutDown()
> {
>-    RELEASE_ASSERT(RunLoop::isMain());
>+    RELEASE_ASSERT(isMainThreadOrCheckDisabled());
> 
>     shutDownProcess();
> 
>@@ -253,7 +265,7 @@ WebPageProxy* WebProcessProxy::webPage(uint64_t pageID)
> void WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPersistentDataStores(OptionSet<WebsiteDataType> dataTypes, Vector<String>&& topPrivatelyControlledDomains, bool shouldNotifyPage, CompletionHandler<void (const HashSet<String>&)>&& completionHandler)
> {
>     // We expect this to be called on the main thread so we get the default website data store.
>-    ASSERT(RunLoop::isMain());
>+    ASSERT(isMainThreadOrCheckDisabled());
>     
>     struct CallbackAggregator : ThreadSafeRefCounted<CallbackAggregator> {
>         explicit CallbackAggregator(CompletionHandler<void(HashSet<String>)>&& completionHandler)
>@@ -301,7 +313,7 @@ void WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPers
>         callbackAggregator->addPendingCallback();
>         dataStore.removeDataForTopPrivatelyControlledDomains(dataTypes, fetchOptions, topPrivatelyControlledDomains, [callbackAggregator, shouldNotifyPage, page](HashSet<String>&& domainsWithDeletedWebsiteData) {
>             // When completing the task, we should be getting called on the main thread.
>-            ASSERT(RunLoop::isMain());
>+            ASSERT(isMainThreadOrCheckDisabled());
>             
>             if (shouldNotifyPage)
>                 page.value->postMessageToInjectedBundle("WebsiteDataDeletionForTopPrivatelyOwnedDomainsFinished", nullptr);
>@@ -315,7 +327,7 @@ void WebProcessProxy::deleteWebsiteDataForTopPrivatelyControlledDomainsInAllPers
> void WebProcessProxy::topPrivatelyControlledDomainsWithWebsiteData(OptionSet<WebsiteDataType> dataTypes, bool shouldNotifyPage, CompletionHandler<void(HashSet<String>&&)>&& completionHandler)
> {
>     // We expect this to be called on the main thread so we get the default website data store.
>-    ASSERT(RunLoop::isMain());
>+    ASSERT(isMainThreadOrCheckDisabled());
>     
>     struct CallbackAggregator : ThreadSafeRefCounted<CallbackAggregator> {
>         explicit CallbackAggregator(CompletionHandler<void(HashSet<String>&&)>&& completionHandler)
>@@ -363,7 +375,7 @@ void WebProcessProxy::topPrivatelyControlledDomainsWithWebsiteData(OptionSet<Web
>         callbackAggregator->addPendingCallback();
>         dataStore.topPrivatelyControlledDomainsWithWebsiteData(dataTypes, { }, [callbackAggregator, shouldNotifyPage, page = makeRef(*page)](HashSet<String>&& domainsWithDataRecords) {
>             // When completing the task, we should be getting called on the main thread.
>-            ASSERT(RunLoop::isMain());
>+            ASSERT(isMainThreadOrCheckDisabled());
>             
>             if (shouldNotifyPage)
>                 page->postMessageToInjectedBundle("WebsiteDataScanForTopPrivatelyControlledDomainsFinished", nullptr);
>@@ -767,7 +779,7 @@ bool WebProcessProxy::mayBecomeUnresponsive()
> 
> void WebProcessProxy::didFinishLaunching(ProcessLauncher* launcher, IPC::Connection::Identifier connectionIdentifier)
> {
>-    RELEASE_ASSERT(RunLoop::isMain());
>+    RELEASE_ASSERT(isMainThreadOrCheckDisabled());
> 
>     ChildProcessProxy::didFinishLaunching(launcher, connectionIdentifier);
>

Actions: View | Formatted Diff | Diff

Attachments on bug 188621: 347219 | 347220