Attachment 346944 Details for Bug 188480 – Updated per review comment

[patch] Updated per review comment

bug-188480-20180810171915.patch (text/plain), 4.44 KB, created by Ryosuke Niwa on 2018-08-10 17:19:15 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2018-08-10 17:19:15 PDT

Size: 4.44 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 234777)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,14 @@
>+2018-08-10  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        [macOS] Multiple third party apps crash due to the thread safety check in TimerBase::setNextFireTime
>+        https://bugs.webkit.org/show_bug.cgi?id=188480
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added the SDK version for macOS Mojave.
>+
>+        * wtf/spi/darwin/dyldSPI.h:
>+
> 2018-08-10  Antti Koivisto  <antti@apple.com>
> 
>         Use OptionSet for various RenderLayer flags
>Index: Source/WTF/wtf/spi/darwin/dyldSPI.h
>===================================================================
>--- Source/WTF/wtf/spi/darwin/dyldSPI.h	(revision 234772)
>+++ Source/WTF/wtf/spi/darwin/dyldSPI.h	(working copy)
>@@ -45,6 +45,10 @@
> #define DYLD_MACOSX_VERSION_10_13 0x000A0D00
> #endif
> 
>+#ifndef DYLD_MACOSX_VERSION_10_14
>+#define DYLD_MACOSX_VERSION_10_14 0x000A0E00
>+#endif
>+
> #else
> 
> #define DYLD_IOS_VERSION_3_0 0x00030000
>@@ -61,6 +65,7 @@
> #define DYLD_MACOSX_VERSION_10_11 0x000A0B00
> #define DYLD_MACOSX_VERSION_10_12 0x000A0C00
> #define DYLD_MACOSX_VERSION_10_13 0x000A0D00
>+#define DYLD_MACOSX_VERSION_10_14 0x000A0E00
> 
> #endif
> 
>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 234772)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,20 @@
>+2018-08-10  Ryosuke Niwa  <rniwa@webkit.org>
>+
>+        [macOS] Multiple third party apps crash due to the thread safety check in TimerBase::setNextFireTime
>+        https://bugs.webkit.org/show_bug.cgi?id=188480
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Suppress the release assert in WebKit1 on macOS (isInWebProcess is always true in non-Cocoa platforms).
>+
>+        In the future, we should consider throwing Objective-C exceptions when third party apps call WebKit1
>+        or WebKit2 APIs in non-main threads.
>+
>+        * platform/Timer.cpp:
>+        (WebCore::shouldSuppressThreadSafetyCheck): Extracted out of ~TimerBase and setNextFireTime.
>+        (WebCore::TimerBase::~TimerBase):
>+        (WebCore::TimerBase::setNextFireTime):
>+
> 2018-08-10  Daniel Bates  <dabates@apple.com>
> 
>         Cleanup: Remove unnecessary code to resume animations from CachedFrameBase::restore()
>Index: Source/WebCore/platform/Timer.cpp
>===================================================================
>--- Source/WebCore/platform/Timer.cpp	(revision 234772)
>+++ Source/WebCore/platform/Timer.cpp	(working copy)
>@@ -27,6 +27,7 @@
> #include "config.h"
> #include "Timer.h"
> 
>+#include "RuntimeApplicationChecks.h"
> #include "SharedTimer.h"
> #include "ThreadGlobalData.h"
> #include "ThreadTimers.h"
>@@ -36,6 +37,10 @@
> #include <wtf/MainThread.h>
> #include <wtf/Vector.h>
> 
>+#if USE(WEB_THREAD) || PLATFORM(MAC)
>+#include <wtf/spi/darwin/dyldSPI.h>
>+#endif
>+
> namespace WebCore {
> 
> class TimerHeapReference;
>@@ -184,6 +189,17 @@ inline bool TimerHeapLessThanFunction::o
> 
> // ----------------
> 
>+static bool shouldSuppressThreadSafetyCheck()
>+{
>+#if USE(WEB_THREAD)
>+    return WebThreadIsEnabled();
>+#elif PLATFORM(MAC)
>+    return !isInWebProcess() && applicationSDKVersion() < DYLD_MACOSX_VERSION_10_14;
>+#else
>+    return false;
>+#endif
>+}
>+
> TimerBase::TimerBase()
>     : m_heapIndex(-1)
>     , m_wasDeleted(false)
>@@ -192,12 +208,8 @@ TimerBase::TimerBase()
> 
> TimerBase::~TimerBase()
> {
>-#if USE(WEB_THREAD)
>     ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
>-    RELEASE_ASSERT(WebThreadIsEnabled() || canAccessThreadLocalDataForThread(m_thread.get()));
>-#else
>-    RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
>-#endif
>+    RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
>     stop();
>     ASSERT(!inHeap());
>     m_wasDeleted = true;
>@@ -364,12 +376,8 @@ void TimerBase::updateHeapIfNeeded(Monot
> 
> void TimerBase::setNextFireTime(MonotonicTime newTime)
> {
>-#if USE(WEB_THREAD)
>     ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
>-    RELEASE_ASSERT(WebThreadIsEnabled() || canAccessThreadLocalDataForThread(m_thread.get()));
>-#else
>-    RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()));
>-#endif
>+    RELEASE_ASSERT(canAccessThreadLocalDataForThread(m_thread.get()) || shouldSuppressThreadSafetyCheck());
>     RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!m_wasDeleted);
> 
>     if (m_unalignedNextFireTime != newTime)

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188480: 346935 | 346944