Changeset 284226 in webkit

Timestamp:

Oct 14, 2021, 7:02:14 PM (4 years ago)

Author:

Chris Dumez

Message:

Service workers running on the main thread should use the main VM
​https://bugs.webkit.org/show_bug.cgi?id=231753

Reviewed by Geoffrey Garen.

Source/WebCore:

ervice workers running on the main thread should use the main VM. This makes life easier for injected
bundle clients and there is no strong reason to use a separate VM since VMs are mainly used for thread
safety / isolation.

No new tests, extended existing API test.

• workers/WorkerOrWorkletGlobalScope.cpp:

(WebCore::WorkerOrWorkletGlobalScope::isContextThread const):
WorkerOrWorkletThread::thread() returns null when the service worker is running on the main thread.
Update WorkerOrWorkletGlobalScope::isContextThread() to deal with that and properly treats the
main thread as the context thread in this case.

• workers/WorkerOrWorkletScriptController.cpp:

(WebCore::WorkerOrWorkletScriptController::WorkerOrWorkletScriptController):
(WebCore::WorkerOrWorkletScriptController::scheduleExecutionTermination):

Tools:

Extend API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerPagePlugIn.mm:

(-[ServiceWorkerPagePlugIn webProcessPlugInBrowserContextController:serviceWorkerGlobalObjectIsAvailableForFrame:inScriptWorld:]):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/workers/WorkerGlobalScope.cpp (modified) (2 diffs)
• Source/WebCore/workers/WorkerOrWorkletGlobalScope.cpp (modified) (1 diff)
• Source/WebCore/workers/WorkerOrWorkletScriptController.cpp (modified) (3 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerPagePlugIn.mm (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-10-14  Chris Dumez  <cdumez@apple.com>
+
+        Service workers running on the main thread should use the main VM
+        https://bugs.webkit.org/show_bug.cgi?id=231753
+
+        Reviewed by Geoffrey Garen.
+
+        ervice workers running on the main thread should use the main VM. This makes life easier for injected
+        bundle clients and there is no strong reason to use a separate VM since VMs are mainly used for thread
+        safety / isolation.
+
+        No new tests, extended existing API test.
+
+        * workers/WorkerOrWorkletGlobalScope.cpp:
+        (WebCore::WorkerOrWorkletGlobalScope::isContextThread const):
+        WorkerOrWorkletThread::thread() returns null when the service worker is running on the main thread.
+        Update WorkerOrWorkletGlobalScope::isContextThread() to deal with that and properly treats the
+        main thread as the context thread in this case.
+
+        * workers/WorkerOrWorkletScriptController.cpp:
+        (WebCore::WorkerOrWorkletScriptController::WorkerOrWorkletScriptController):
+        (WebCore::WorkerOrWorkletScriptController::scheduleExecutionTermination):
+
 2021-10-14  Aditya Keerthi  <akeerthi@apple.com>
 

trunk/Source/WebCore/workers/WorkerGlobalScope.cpp

@@ -32 +32 @@
 #include "CSSValueList.h"
 #include "CSSValuePool.h"
+#include "CommonVM.h"
 #include "ContentSecurityPolicy.h"
 #include "Crypto.h"
@@ -80 +81 @@
 
 WorkerGlobalScope::WorkerGlobalScope(WorkerThreadType type, const WorkerParameters& params, Ref<SecurityOrigin>&& origin, WorkerThread& thread, Ref<SecurityOrigin>&& topOrigin, IDBClient::IDBConnectionProxy* connectionProxy, SocketProvider* socketProvider)
-    : WorkerOrWorkletGlobalScope(type, JSC::VM::create(), &thread)
+    : WorkerOrWorkletGlobalScope(type, isMainThread() ? Ref { commonVM() } : JSC::VM::create(), &thread)
     , m_url(params.scriptURL)
     , m_identifier(params.identifier)

trunk/Source/WebCore/workers/WorkerOrWorkletGlobalScope.cpp

@@ -112 +112 @@
 {
     auto* thread = workerOrWorkletThread();
-    return thread ? thread->thread() == &Thread::current() : isMainThread();
+    return thread && thread->thread() ? thread->thread() == &Thread::current() : isMainThread();
 }
 

trunk/Source/WebCore/workers/WorkerOrWorkletScriptController.cpp

@@ -28 +28 @@
 #include "WorkerOrWorkletScriptController.h"
 
+#include "CommonVM.h"
 #include "DedicatedWorkerGlobalScope.h"
 #include "EventLoop.h"
@@ -69 +70 @@
     , m_globalScopeWrapper(*m_vm)
 {
-    m_vm->heap.acquireAccess(); // It's not clear that we have good discipline for heap access, so turn it on permanently.
-    {
-        JSLockHolder lock(m_vm.get());
-        m_vm->ensureTerminationException();
-    }
-
-    JSVMClientData::initNormalWorld(m_vm.get(), type);
+    if (!isMainThread() || m_vm != &commonVM()) {
+        m_vm->heap.acquireAccess(); // It's not clear that we have good discipline for heap access, so turn it on permanently.
+        {
+            JSLockHolder lock(m_vm.get());
+            m_vm->ensureTerminationException();
+        }
+
+        JSVMClientData::initNormalWorld(m_vm.get(), type);
+    }
 }
 
@@ -130 +133 @@
         m_isTerminatingExecution = true;
     }
-    m_vm->notifyNeedTermination();
+    if (m_vm != &commonVM())
+        m_vm->notifyNeedTermination();
 }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2021-10-14  Chris Dumez  <cdumez@apple.com>
+
+        Service workers running on the main thread should use the main VM
+        https://bugs.webkit.org/show_bug.cgi?id=231753
+
+        Reviewed by Geoffrey Garen.
+
+        Extend API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerPagePlugIn.mm:
+        (-[ServiceWorkerPagePlugIn webProcessPlugInBrowserContextController:serviceWorkerGlobalObjectIsAvailableForFrame:inScriptWorld:]):
+
 2021-10-14  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerPagePlugIn.mm

@@ -27 +27 @@
 
 #import "ServiceWorkerPageProtocol.h"
+#import <JavaScriptCore/JSContextRef.h>
 #import <WebKit/WKBundlePage.h>
 #import <WebKit/WKWebProcessPlugIn.h>
@@ -33 +34 @@
 #import <WebKit/WKWebProcessPlugInFramePrivate.h>
 #import <WebKit/WKWebProcessPlugInLoadDelegate.h>
+#import <WebKit/WKWebProcessPlugInScriptWorld.h>
 #import <WebKit/_WKRemoteObjectInterface.h>
 #import <WebKit/_WKRemoteObjectRegistry.h>
@@ -48 +50 @@
     RELEASE_ASSERT(RunLoop::isMain());
     RELEASE_ASSERT(frame);
-    JSContext *jsContext = [frame jsContextForServiceWorkerWorld:scriptWorld];
-    RELEASE_ASSERT(jsContext);
-    RELEASE_ASSERT([WKWebProcessPlugInFrame lookUpFrameFromJSContext:jsContext] == frame);
+    JSContext *serviceWorkerJSContext = [frame jsContextForServiceWorkerWorld:scriptWorld];
+    RELEASE_ASSERT(serviceWorkerJSContext);
+    RELEASE_ASSERT([WKWebProcessPlugInFrame lookUpFrameFromJSContext:serviceWorkerJSContext] == frame);
 
-    JSValue *globalIsServiceWorkerGlobalScope = [jsContext evaluateScript:@"self.__proto__ === ServiceWorkerGlobalScope.prototype"];
+    JSContext *mainFrameJSContext = [frame jsContextForWorld:[WKWebProcessPlugInScriptWorld normalWorld]];
+    RELEASE_ASSERT(mainFrameJSContext);
+
+    // The main frame and the service worker should have different JSContexts but should use the same VM.
+    RELEASE_ASSERT(mainFrameJSContext != serviceWorkerJSContext);
+    RELEASE_ASSERT(JSContextGetGroup(mainFrameJSContext.JSGlobalContextRef) == JSContextGetGroup(serviceWorkerJSContext.JSGlobalContextRef));
+
+    RELEASE_ASSERT(scriptWorld == [WKWebProcessPlugInScriptWorld normalWorld]);
+
+    JSValue *globalIsServiceWorkerGlobalScope = [serviceWorkerJSContext evaluateScript:@"self.__proto__ === ServiceWorkerGlobalScope.prototype"];
     RELEASE_ASSERT(!!globalIsServiceWorkerGlobalScope);
     RELEASE_ASSERT([globalIsServiceWorkerGlobalScope toBool]);