Attachment 372319 Details for Bug 198911 – Patch

[patch] Patch

bug-198911-20190617211832.patch (text/plain), 13.10 KB, created by Yusuke Suzuki on 2019-06-17 21:18:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-06-17 21:18:33 PDT

Size: 13.10 KB

patch

obsolete

>Subversion Revision: 246505
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 99c24f8d9a9599b3bd310448df41d183b858800e..eaf39cb5694820838cd3492af25514a907696a21 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-06-17  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] JSLock should be WebThread aware
>+        https://bugs.webkit.org/show_bug.cgi?id=198911
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Since WebKitLegacy content rendering is driven in web thread instead of main thread in iOS, user of WebKitLegacy (e.g. UIWebView) needs
>+        to grab the web thread lock (which is a recursive lock) from the main thread before touching that. WebKitLegacy can expose JSContext
>+        for the web view, and we can interact with the JS content through JavaScriptCore APIs. However, since web thread is a concept in WebCore,
>+        JavaScriptCore APIs do not grab the web thread lock. As a result, WebKitLegacy web content can be modified from the main thread and the
>+        web thread simultaneously.
>+
>+        This patch makes JSC aware of WebThread: JSLock grabs the web thread lock before grabbing JS's lock. Since both JSLock and the web thread
>+        lock are recursive locks, nested lock is totally OK. Only the problem is the order of locking, and we always grab locks in (1) the web
>+        thread lock and (2) JSLock order.
>+
>+        * API/JSVirtualMachine.mm:
>+        (-[JSVirtualMachine isWebThreadAware]):
>+        * API/JSVirtualMachineInternal.h:
>+        * runtime/JSLock.cpp:
>+        (JSC::JSLock::lock):
>+        * runtime/JSLock.h:
>+        (JSC::JSLock::makeWebThreadAware):
>+        (JSC::JSLock::isWebThreadAware const):
>+
> 2019-06-17  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Introduce DisposableCallSiteIndex to enforce type-safety
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c9aed570f939ddec8e12986e9712f8dade127c0b..1cd6233839b7d03398563834ab84f572e191c7f8 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-06-17  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] JSLock should be WebThread aware
>+        https://bugs.webkit.org/show_bug.cgi?id=198911
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * bindings/js/CommonVM.cpp:
>+        (WebCore::commonVMSlow):
>+
> 2019-06-17  Kenneth Russell  <kbr@chromium.org>
> 
>         Support using ANGLE as the backend for the WebGL implementation
>diff --git a/Source/JavaScriptCore/API/JSVirtualMachine.mm b/Source/JavaScriptCore/API/JSVirtualMachine.mm
>index 2afc097b2facd931a447e3be30fb37aadee7f850..62eef4d4892c3d4f09b6323a4f0f2ff8c5c946b6 100644
>--- a/Source/JavaScriptCore/API/JSVirtualMachine.mm
>+++ b/Source/JavaScriptCore/API/JSVirtualMachine.mm
>@@ -248,6 +248,11 @@ - (JSC::VM&)vm
>     return *toJS(m_group);
> }
> 
>+- (BOOL)isWebThreadAware
>+{
>+    return [self vm].apiLock().isWebThreadAware();
>+}
>+
> + (void)setCrashOnVMCreation:(BOOL)shouldCrash
> {
>     JSC::VM::setCrashOnVMCreation(shouldCrash);
>diff --git a/Source/JavaScriptCore/API/JSVirtualMachineInternal.h b/Source/JavaScriptCore/API/JSVirtualMachineInternal.h
>index f64ee1ae028a696a81c58e36c36d450e6c0dbc98..ee86b615339ff67cbc6e3cbf3141d915cbec2606 100644
>--- a/Source/JavaScriptCore/API/JSVirtualMachineInternal.h
>+++ b/Source/JavaScriptCore/API/JSVirtualMachineInternal.h
>@@ -46,6 +46,8 @@ JSContextGroupRef getGroupFromVirtualMachine(JSVirtualMachine *);
> 
> - (JSC::VM&)vm;
> 
>+- (BOOL)isWebThreadAware;
>+
> @end
> 
> #endif // defined(__OBJC__)
>diff --git a/Source/JavaScriptCore/runtime/JSLock.cpp b/Source/JavaScriptCore/runtime/JSLock.cpp
>index 7cc3f69d4afc2e7d52ebcb9739c81d297255c52c..f41ecd7167a02b44f44bf8af113196c7fcdfdace 100644
>--- a/Source/JavaScriptCore/runtime/JSLock.cpp
>+++ b/Source/JavaScriptCore/runtime/JSLock.cpp
>@@ -35,6 +35,10 @@
> #include <wtf/Threading.h>
> #include <wtf/threads/Signals.h>
> 
>+#if USE(WEB_THREAD)
>+#include <wtf/ios/WebCoreThread.h>
>+#endif
>+
> namespace JSC {
> 
> Lock GlobalJSLock::s_sharedInstanceMutex;
>@@ -105,6 +109,13 @@ void JSLock::lock()
> void JSLock::lock(intptr_t lockCount)
> {
>     ASSERT(lockCount > 0);
>+#if USE(WEB_THREAD)
>+    if (m_isWebThreadAware) {
>+        ASSERT(WebCoreWebThreadIsEnabled && WebCoreWebThreadIsEnabled());
>+        WebCoreWebThreadLock();
>+    }
>+#endif
>+
>     bool success = m_lock.tryLock();
>     if (UNLIKELY(!success)) {
>         if (currentThreadIsHoldingLock()) {
>diff --git a/Source/JavaScriptCore/runtime/JSLock.h b/Source/JavaScriptCore/runtime/JSLock.h
>index f461f820f9e3aa5f1ec86a02113da2296f3330e8..1e8758c52642a7ff2b07bf723d68d63bb436f52f 100644
>--- a/Source/JavaScriptCore/runtime/JSLock.h
>+++ b/Source/JavaScriptCore/runtime/JSLock.h
>@@ -119,6 +119,13 @@ class JSLock : public ThreadSafeRefCounted<JSLock> {
>         unsigned m_dropDepth;
>     };
> 
>+    void makeWebThreadAware()
>+    {
>+        m_isWebThreadAware = true;
>+    }
>+
>+    bool isWebThreadAware() const { return m_isWebThreadAware; }
>+
> private:
>     void lock(intptr_t lockCount);
>     void unlock(intptr_t unlockCount);
>@@ -130,6 +137,7 @@ class JSLock : public ThreadSafeRefCounted<JSLock> {
>     void grabAllLocks(DropAllLocks*, unsigned lockCount);
> 
>     Lock m_lock;
>+    bool m_isWebThreadAware { false };
>     // We cannot make m_ownerThread an optional (instead of pairing it with an explicit
>     // m_hasOwnerThread) because currentThreadIsHoldingLock() may be called from a
>     // different thread, and an optional is vulnerable to races.
>diff --git a/Source/WebCore/bindings/js/CommonVM.cpp b/Source/WebCore/bindings/js/CommonVM.cpp
>index c1e94b8b85726b3d51fe11958992351bf37e1b52..09b784f9ed356ad6bcba84f5e63bf8a6f1ea267d 100644
>--- a/Source/WebCore/bindings/js/CommonVM.cpp
>+++ b/Source/WebCore/bindings/js/CommonVM.cpp
>@@ -59,6 +59,8 @@ JSC::VM& commonVMSlow()
>     vm.heap.acquireAccess(); // At any time, we may do things that affect the GC.
> 
> #if PLATFORM(IOS_FAMILY)
>+    if (WebThreadIsEnabled())
>+        vm.apiLock().makeWebThreadAware();
>     vm.setRunLoop(WebThreadRunLoop());
>     vm.heap.machineThreads().addCurrentThread();
> #endif
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index c13be679802887515116c627d9f5ac63582ca456..5fb8580281bb27ad27bdb41a6db29daa9f7ec7d7 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-06-17  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] JSLock should be WebThread aware
>+        https://bugs.webkit.org/show_bug.cgi?id=198911
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
>+        * TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm: Added.
>+        (TestWebKitAPI::TEST):
>+
> 2019-06-17  Brent Fulgham  <bfulgham@apple.com>
> 
>         Ensure ITP state is relayed to Network Process on restart
>diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>index fdbcd968ad33f26db15a4cf4841ed064cd18396c..7a16ebcc83daaa0d0f8db3aae6d25f0e96b64595 100644
>--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
>@@ -875,6 +875,7 @@
> 		E194E1BD177E53C7009C4D4E /* StopLoadingFromDidReceiveResponse.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = E194E1BC177E534A009C4D4E /* StopLoadingFromDidReceiveResponse.html */; };
> 		E324A6F02041C82000A76593 /* UniqueArray.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E398BC0F2041C76300387136 /* UniqueArray.cpp */; };
> 		E32B549222810AC4008AD702 /* Packed.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E32B549122810AC0008AD702 /* Packed.cpp */; };
>+		E35FC7B222B82A7300F32F98 /* JSLockTakesWebThreadLock.mm in Sources */ = {isa = PBXBuildFile; fileRef = E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */; };
> 		E373D7911F2CF35200C6FAAF /* Signals.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E3953F951F2CF32100A76A2E /* Signals.cpp */; };
> 		E38A0D351FD50CC300E98C8B /* Threading.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E38A0D341FD50CBC00E98C8B /* Threading.cpp */; };
> 		E3A1E77F21B25B39008C6007 /* URLParserTextEncoding.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E3A1E77E21B25B39008C6007 /* URLParserTextEncoding.cpp */; };
>@@ -2272,6 +2273,7 @@
> 		E194E1BC177E534A009C4D4E /* StopLoadingFromDidReceiveResponse.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = StopLoadingFromDidReceiveResponse.html; sourceTree = "<group>"; };
> 		E19DB9781B32137C00DB38D4 /* NavigatorLanguage.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = NavigatorLanguage.mm; sourceTree = "<group>"; };
> 		E32B549122810AC0008AD702 /* Packed.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = Packed.cpp; sourceTree = "<group>"; };
>+		E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = JSLockTakesWebThreadLock.mm; sourceTree = "<group>"; };
> 		E388887020C9098100E632BC /* WorkerPool.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WorkerPool.cpp; sourceTree = "<group>"; };
> 		E38A0D341FD50CBC00E98C8B /* Threading.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Threading.cpp; sourceTree = "<group>"; };
> 		E3953F951F2CF32100A76A2E /* Signals.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Signals.cpp; sourceTree = "<group>"; };
>@@ -3807,6 +3809,7 @@
> 			children = (
> 				CDC8E49A1BC728FE00594FEC /* Resources */,
> 				CDC8E4851BC5B19400594FEC /* AudioSessionCategoryIOS.mm */,
>+				E35FC7B122B82A6D00F32F98 /* JSLockTakesWebThreadLock.mm */,
> 				CDC0932A21C872C10030C4B0 /* ScrollingDoesNotPauseMedia.mm */,
> 				0F4FFA9D1ED3AA8500F7111F /* SnapshotViaRenderInContext.mm */,
> 			);
>@@ -4303,6 +4306,7 @@
> 				7CCE7EAD1A411A3400447C4C /* JavaScriptTest.cpp in Sources */,
> 				7CCE7EA51A411A0800447C4C /* JavaScriptTestMac.mm in Sources */,
> 				5C0160C121A132460077FA32 /* JITEnabled.mm in Sources */,
>+				E35FC7B222B82A7300F32F98 /* JSLockTakesWebThreadLock.mm in Sources */,
> 				7CCE7EC41A411A7E00447C4C /* JSWrapperForNodeInWebFrame.mm in Sources */,
> 				F45E15732112CE2900307E82 /* KeyboardInputTestsIOS.mm in Sources */,
> 				7CCE7F061A411AE600447C4C /* LayoutMilestonesWithAllContentInFrame.cpp in Sources */,
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm b/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm
>new file mode 100644
>index 0000000000000000000000000000000000000000..dcbd114403658fafacf90c6fc79a3a0a1e380c8e
>--- /dev/null
>+++ b/Tools/TestWebKitAPI/Tests/WebKitLegacy/ios/JSLockTakesWebThreadLock.mm
>@@ -0,0 +1,58 @@
>+/*
>+ * Copyright (C) 2019 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#import "config.h"
>+
>+#if PLATFORM(IOS_FAMILY)
>+
>+#import "PlatformUtilities.h"
>+#import <JavaScriptCore/JSVirtualMachine.h>
>+#import <JavaScriptCore/JSVirtualMachineInternal.h>
>+#import <UIKit/UIKit.h>
>+#import <stdlib.h>
>+#import <wtf/RetainPtr.h>
>+
>+namespace TestWebKitAPI {
>+
>+TEST(WebKitLegacy, JSLockTakesWebThreadLock)
>+{
>+    RetainPtr<UIWindow> uiWindow = adoptNS([[UIWindow alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
>+    RetainPtr<UIWebView> uiWebView = adoptNS([[UIWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
>+    [uiWindow addSubview:uiWebView.get()];
>+    [uiWebView loadHTMLString:@"<p>WebThreadLock and JSLock</p>" baseURL:nil];
>+    RetainPtr<JSContext> jsContext = [uiWebView valueForKeyPath:@"documentView.webView.mainFrame.javaScriptContext"];
>+
>+    EXPECT_TRUE(!!jsContext.get());
>+
>+    RetainPtr<JSVirtualMachine> jsVM = [jsContext virtualMachine];
>+    EXPECT_TRUE([jsVM isWebThreadAware]);
>+
>+    // XMLHttpRequest has Timer, which has thread afinity.
>+    [jsContext evaluateScript:@"for (var i = 0; i < 1e5; ++i) new XMLHttpRequest;"];
>+}
>+
>+}
>+
>+#endif

Actions: View | Formatted Diff | Diff

Attachments on bug 198911: 372236 | 372237 | 372261 | 372303 | 372319 | 372329 | 372331