[iOS Sim Debug] ASSERTION FAILED The atomic string comes from an othe…

…r thread! Layout Test imported/w3c/web-platform-tests/workers/WorkerNavigator_appName.htm is a flaky crash https://bugs.webkit.org/show_bug.cgi?id=197530 <rdar://problem/50448285> Reviewed by Geoffrey Garen. The issue is that NavigatorBase::platform() was not thread safe but was called by both Navigator on the main thread and WorkerNavigator on worker threads. No new tests, covered by existing tests. * page/Navigator.cpp: (WebCore::Navigator::platform const): * page/Navigator.h: * page/NavigatorBase.cpp: (WebCore::NavigatorBase::platform const): * page/NavigatorBase.h: Make NavigatorBase::platform() thread safe. * platform/ios/Device.cpp: (WebCore::deviceName): * platform/ios/Device.h: Make WebCore::deviceName() thread safe. * platform/ios/UserAgentIOS.mm: (WebCore::deviceNameForUserAgent): Cache value returned by WebCore::deviceName() for performance. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@244927 268f45cc-cd09-0410-ab3c-d52691b4dbfc
haiku · May 3, 2019 · 878faaa · 878faaa
1 parent f998a7e
commit 878faaa
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 36 deletions.
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,34 @@
+2019-05-03  Chris Dumez  <cdumez@apple.com>
+
+        [iOS Sim Debug] ASSERTION FAILED The atomic string comes from an other thread! Layout Test imported/w3c/web-platform-tests/workers/WorkerNavigator_appName.htm is a flaky crash
+        https://bugs.webkit.org/show_bug.cgi?id=197530
+        <rdar://problem/50448285>
+
+        Reviewed by Geoffrey Garen.
+
+        The issue is that NavigatorBase::platform() was not thread safe but was called by both Navigator on
+        the main thread and WorkerNavigator on worker threads.
+
+        No new tests, covered by existing tests.
+
+        * page/Navigator.cpp:
+        (WebCore::Navigator::platform const):
+        * page/Navigator.h:
+
+        * page/NavigatorBase.cpp:
+        (WebCore::NavigatorBase::platform const):
+        * page/NavigatorBase.h:
+        Make NavigatorBase::platform() thread safe.
+
+        * platform/ios/Device.cpp:
+        (WebCore::deviceName):
+        * platform/ios/Device.h:
+        Make WebCore::deviceName() thread safe.
+
+        * platform/ios/UserAgentIOS.mm:
+        (WebCore::deviceNameForUserAgent):
+        Cache value returned by WebCore::deviceName() for performance.
+
 2019-05-03  Chris Dumez  <cdumez@apple.com>
 
         Use WeakPtr for JSLazyEventListener::m_originalNode for safety

diff --git a/Source/WebCore/page/Navigator.cpp b/Source/WebCore/page/Navigator.cpp
@@ -81,7 +81,7 @@ const String& Navigator::userAgent() const
     return m_userAgent;
 }
 
-const String& Navigator::platform() const
+String Navigator::platform() const
 {
     auto* frame = this->frame();
     if (!frame || !frame->page())

diff --git a/Source/WebCore/page/Navigator.h b/Source/WebCore/page/Navigator.h
@@ -44,7 +44,7 @@ class Navigator final : public NavigatorBase, public ScriptWrappable, public DOM
     bool cookieEnabled() const;
     bool javaEnabled() const;
     const String& userAgent() const final;
-    const String& platform() const final;
+    String platform() const final;
     void userAgentChanged();
     bool onLine() const final;
     void share(ScriptExecutionContext&, ShareData, Ref<DeferredPromise>&&);

diff --git a/Source/WebCore/page/NavigatorBase.cpp b/Source/WebCore/page/NavigatorBase.cpp
@@ -45,20 +45,6 @@
 #include "Device.h"
 #endif
 
-#ifndef WEBCORE_NAVIGATOR_PLATFORM
-#if PLATFORM(IOS_FAMILY)
-#define WEBCORE_NAVIGATOR_PLATFORM deviceName()
-#elif OS(MAC_OS_X) && (CPU(PPC) || CPU(PPC64))
-#define WEBCORE_NAVIGATOR_PLATFORM "MacPPC"_s
-#elif OS(MAC_OS_X) && (CPU(X86) || CPU(X86_64))
-#define WEBCORE_NAVIGATOR_PLATFORM "MacIntel"_s
-#elif OS(WINDOWS)
-#define WEBCORE_NAVIGATOR_PLATFORM "Win32"_s
-#else
-#define WEBCORE_NAVIGATOR_PLATFORM emptyString()
-#endif
-#endif // ifndef WEBCORE_NAVIGATOR_PLATFORM
-
 #ifndef WEBCORE_NAVIGATOR_PRODUCT
 #define WEBCORE_NAVIGATOR_PRODUCT "Gecko"_s
 #endif // ifndef WEBCORE_NAVIGATOR_PRODUCT
@@ -96,17 +82,26 @@ String NavigatorBase::appVersion() const
     return agent.substring(agent.find('/') + 1);
 }
 
-const String& NavigatorBase::platform() const
+String NavigatorBase::platform() const
 {
-    static NeverDestroyed<String> defaultPlatform = WEBCORE_NAVIGATOR_PLATFORM;
 #if OS(LINUX)
-    if (!String(WEBCORE_NAVIGATOR_PLATFORM).isEmpty())
-        return defaultPlatform;
-    struct utsname osname;
-    static NeverDestroyed<String> platformName(uname(&osname) >= 0 ? String(osname.sysname) + " "_str + String(osname.machine) : emptyString());
-    return platformName;
+    static LazyNeverDestroyed<String> platformName;
+    static std::once_flag onceKey;
+    std::call_once(onceKey, [] {
+        struct utsname osname;
+        platformName.construct(uname(&osname) >= 0 ? String(osname.sysname) + " "_str + String(osname.machine) : String(""_s));
+    });
+    return platformName->isolatedCopy();
+#elif PLATFORM(IOS_FAMILY)
+    return deviceName();
+#elif OS(MAC_OS_X) && (CPU(PPC) || CPU(PPC64))
+    return "MacPPC"_s;
+#elif OS(MAC_OS_X) && (CPU(X86) || CPU(X86_64))
+    return "MacIntel"_s;
+#elif OS(WINDOWS)
+    return "Win32"_s;
 #else
-    return defaultPlatform;
+    return ""_s;
 #endif
 }
 

diff --git a/Source/WebCore/page/NavigatorBase.h b/Source/WebCore/page/NavigatorBase.h
@@ -44,7 +44,7 @@ class NavigatorBase : public RefCounted<NavigatorBase>, public ContextDestructio
     static String appName();
     String appVersion() const;
     virtual const String& userAgent() const = 0;
-    virtual const String& platform() const;
+    virtual String platform() const;
 
     static String appCodeName();
     static String product();

diff --git a/Source/WebCore/platform/ios/Device.cpp b/Source/WebCore/platform/ios/Device.cpp
@@ -54,14 +54,18 @@ MGDeviceClass deviceClass()
     return deviceClass;
 }
 
-const String& deviceName()
+String deviceName()
 {
 #if TARGET_OS_IOS
-    static const NeverDestroyed<String> deviceName = adoptCF(static_cast<CFStringRef>(MGCopyAnswer(kMGQDeviceName, nullptr))).get();
+    static CFStringRef deviceName;
+    static std::once_flag onceKey;
+    std::call_once(onceKey, [] {
+        deviceName = static_cast<CFStringRef>(MGCopyAnswer(kMGQDeviceName, nullptr));
+    });
+    return deviceName;
 #else
-    static const NeverDestroyed<String> deviceName { "iPhone"_s };
+    return "iPhone"_s;
 #endif
-    return deviceName;
 }
 
 bool deviceHasIPadCapability()

diff --git a/Source/WebCore/platform/ios/Device.h b/Source/WebCore/platform/ios/Device.h
@@ -34,7 +34,7 @@
 namespace WebCore {
 
 WEBCORE_EXPORT MGDeviceClass deviceClass();
-const WTF::String& deviceName();
+String deviceName(); // Thread-safe.
 
 // FIXME: Isn't this the same as deviceClass() == MGDeviceClassiPad?
 bool deviceHasIPadCapability();

diff --git a/Source/WebCore/platform/ios/UserAgentIOS.mm b/Source/WebCore/platform/ios/UserAgentIOS.mm
@@ -63,16 +63,19 @@ static inline String deviceNameForUserAgent()
 {
     if (isClassic()) {
         if (isClassicPad())
-            return "iPad";
-        return "iPhone";
+            return "iPad"_s;
+        return "iPhone"_s;
     }
 
-    String name = deviceName();
+    static NeverDestroyed<String> name = [] {
+        auto name = deviceName();
 #if PLATFORM(IOS_FAMILY_SIMULATOR)
-    size_t location = name.find(" Simulator");
-    if (location != notFound) 
-        return name.substring(0, location);
+        size_t location = name.find(" Simulator");
+        if (location != notFound)
+            return name.substring(0, location);
 #endif
+        return name;
+    }();
     return name;
 }