Attachment 368969 Details for Bug 197530 – Patch

[patch] Patch

bug-197530-20190503130616.patch (text/plain), 7.74 KB, created by Chris Dumez on 2019-05-03 13:06:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-05-03 13:06:17 PDT

Size: 7.74 KB

patch

obsolete

>Subversion Revision: 244912
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9a74e09f9f9a019120975b7f2e7842a83e903104..d0b4a13dd00e6b18b38862f388743c1ad11ea158 100644
>--- 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 NOBODY (OOPS!).
>+
>+        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  Antti Koivisto  <antti@apple.com>
> 
>         Add a quirk to make youtube navigation bar scrollable without mouse hover on iOS
>diff --git a/Source/WebCore/page/Navigator.cpp b/Source/WebCore/page/Navigator.cpp
>index 07d045b3fd3eda2402b1f524cbd14f7963a1ad5b..9eb546a7c01f36333ad40eb57e1f7059e5976e7d 100644
>--- 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
>index 7b871084d25eb5541c5b1eb4cd067fa1fe10db22..c09f9e0ac21ac3b2fcc363f3d231c391db9f545a 100644
>--- a/Source/WebCore/page/Navigator.h
>+++ b/Source/WebCore/page/Navigator.h
>@@ -44,7 +44,7 @@ public:
>     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
>index c0136855748e9045a2db848e24a9a5e3e23c81b7..215e6129531fae7788090a8e4510092146b95dce 100644
>--- 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
>index 1a06c4e8ad59401f7a08f90542d1eb99676181a1..d3f3fdabe9c2ea17c3230634f3da8bf3fe704d25 100644
>--- a/Source/WebCore/page/NavigatorBase.h
>+++ b/Source/WebCore/page/NavigatorBase.h
>@@ -44,7 +44,7 @@ public:
>     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
>index 8712cbc5cd7eedd0b4822029f6d0677704e01baa..c65c9657bf148c4916005ad5c95de26384945ea8 100644
>--- 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
>index f906eb9aeea797e917db9b21f9b4ef8ae8762f5f..1990191dd84a0b9ed545576a8d9d12d414803a24 100644
>--- 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
>index 1a9a1a9e6bf25a08ef6e0778d7caae90885b5f92..d883510575ab5e5cfa30e3fb2c7972d233d7ad7e 100644
>--- 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;
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 197530: 368968 | 368969