Plumbing for handling SW scripts failing to evaluate
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Nov 2017 21:23:52 +0000 (21:23 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Nov 2017 21:23:52 +0000 (21:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178926

Reviewed by Chris Dumez.

Source/WebCore:

No new tests (Currently no observable behavior change).

In an upcoming patch we'll actually run the appropriate observable steps for when
a ServiceWorker script fails to evaluate.

This is a standalone refactoring + plumbing patch that will make the observable changes
easier to review.

* bindings/js/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::evaluate):
* bindings/js/WorkerScriptController.h:
(WebCore::WorkerScriptController::workerGlobalScopeWrapper):
(WebCore::WorkerScriptController::vm):
(WebCore::WorkerScriptController::initScriptIfNeeded):

* workers/WorkerMessagingProxy.cpp:
(WebCore::WorkerMessagingProxy::startWorkerGlobalScope):

* workers/WorkerThread.cpp:
(WebCore::WorkerThread::start):
(WebCore::WorkerThread::workerThread):
* workers/WorkerThread.h:

* workers/service/context/SWContextManager.cpp:
(WebCore::SWContextManager::registerServiceWorkerThreadForUpdate):
(WebCore::SWContextManager::registerServiceWorkerThread): Deleted.
* workers/service/context/SWContextManager.h:

* workers/service/context/ServiceWorkerThread.h:
(WebCore::ServiceWorkerThread::serverConnectionIdentifier const):
(WebCore::ServiceWorkerThread::contextData const):

* workers/service/context/ServiceWorkerThreadProxy.cpp:
(WebCore::ServiceWorkerThreadProxy::create):
(WebCore::ServiceWorkerThreadProxy::ServiceWorkerThreadProxy):

* workers/service/server/SWServer.cpp:
(WebCore::SWServer::updateWorker):
(WebCore::SWServer::createWorker): Deleted.
* workers/service/server/SWServer.h:

* workers/service/server/SWServerRegistration.cpp:
(WebCore::SWServerRegistration::scriptFetchFinished):
(WebCore::SWServerRegistration::scriptContextFailedToStart):

Source/WebKit:

* StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
(WebKit::WebSWServerConnection::updateServiceWorkerContext):
(WebKit::WebSWServerConnection::setContextConnection):
(WebKit::WebSWServerConnection::startServiceWorkerContext): Deleted.
* StorageProcess/ServiceWorker/WebSWServerConnection.h:

* WebProcess/Storage/WebSWContextManagerConnection.cpp:
(WebKit::WebSWContextManagerConnection::updateServiceWorker):
(WebKit::WebSWContextManagerConnection::serviceWorkerStartedWithMessage):
(WebKit::WebSWContextManagerConnection::startServiceWorker): Deleted.
* WebProcess/Storage/WebSWContextManagerConnection.h:
* WebProcess/Storage/WebSWContextManagerConnection.messages.in:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224295 268f45cc-cd09-0410-ab3c-d52691b4dbfc

19 files changed:
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/WorkerScriptController.cpp
Source/WebCore/bindings/js/WorkerScriptController.h
Source/WebCore/workers/WorkerMessagingProxy.cpp
Source/WebCore/workers/WorkerThread.cpp
Source/WebCore/workers/WorkerThread.h
Source/WebCore/workers/service/context/SWContextManager.cpp
Source/WebCore/workers/service/context/SWContextManager.h
Source/WebCore/workers/service/context/ServiceWorkerThread.h
Source/WebCore/workers/service/context/ServiceWorkerThreadProxy.cpp
Source/WebCore/workers/service/server/SWServer.cpp
Source/WebCore/workers/service/server/SWServer.h
Source/WebCore/workers/service/server/SWServerRegistration.cpp
Source/WebKit/ChangeLog
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp
Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.h
Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.messages.in

index c64fc18..09fbb59 100644 (file)
@@ -1,3 +1,55 @@
+2017-11-01  Brady Eidson  <beidson@apple.com>
+
+        Plumbing for handling SW scripts failing to evaluate
+        https://bugs.webkit.org/show_bug.cgi?id=178926
+
+        Reviewed by Chris Dumez.
+
+        No new tests (Currently no observable behavior change).
+
+        In an upcoming patch we'll actually run the appropriate observable steps for when
+        a ServiceWorker script fails to evaluate.
+
+        This is a standalone refactoring + plumbing patch that will make the observable changes
+        easier to review.
+
+        * bindings/js/WorkerScriptController.cpp:
+        (WebCore::WorkerScriptController::evaluate):
+        * bindings/js/WorkerScriptController.h:
+        (WebCore::WorkerScriptController::workerGlobalScopeWrapper):
+        (WebCore::WorkerScriptController::vm):
+        (WebCore::WorkerScriptController::initScriptIfNeeded):
+
+        * workers/WorkerMessagingProxy.cpp:
+        (WebCore::WorkerMessagingProxy::startWorkerGlobalScope):
+
+        * workers/WorkerThread.cpp:
+        (WebCore::WorkerThread::start):
+        (WebCore::WorkerThread::workerThread):
+        * workers/WorkerThread.h:
+
+        * workers/service/context/SWContextManager.cpp:
+        (WebCore::SWContextManager::registerServiceWorkerThreadForUpdate):
+        (WebCore::SWContextManager::registerServiceWorkerThread): Deleted.
+        * workers/service/context/SWContextManager.h:
+
+        * workers/service/context/ServiceWorkerThread.h:
+        (WebCore::ServiceWorkerThread::serverConnectionIdentifier const):
+        (WebCore::ServiceWorkerThread::contextData const):
+
+        * workers/service/context/ServiceWorkerThreadProxy.cpp:
+        (WebCore::ServiceWorkerThreadProxy::create):
+        (WebCore::ServiceWorkerThreadProxy::ServiceWorkerThreadProxy):
+
+        * workers/service/server/SWServer.cpp:
+        (WebCore::SWServer::updateWorker):
+        (WebCore::SWServer::createWorker): Deleted.
+        * workers/service/server/SWServer.h:
+
+        * workers/service/server/SWServerRegistration.cpp:
+        (WebCore::SWServerRegistration::scriptFetchFinished):
+        (WebCore::SWServerRegistration::scriptContextFailedToStart):
+
 2017-11-01  Ryosuke Niwa  <rniwa@webkit.org>
 
         Assert that NoEventDispatchAssertion is not in the stack when executing a script
index 9f32811..41cbec0 100644 (file)
@@ -120,20 +120,20 @@ void WorkerScriptController::initScript()
     m_workerGlobalScopeWrapper->setConsoleClient(m_consoleClient.get());
 }
 
-void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode)
+void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, String* returnedExceptionMessage)
 {
     if (isExecutionForbidden())
         return;
 
     NakedPtr<JSC::Exception> exception;
-    evaluate(sourceCode, exception);
+    evaluate(sourceCode, exception, returnedExceptionMessage);
     if (exception) {
         JSLockHolder lock(vm());
         reportException(m_workerGlobalScopeWrapper->globalExec(), exception);
     }
 }
 
-void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, NakedPtr<JSC::Exception>& returnedException)
+void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, NakedPtr<JSC::Exception>& returnedException, String* returnedExceptionMessage)
 {
     if (isExecutionForbidden())
         return;
@@ -157,8 +157,11 @@ void WorkerScriptController::evaluate(const ScriptSourceCode& sourceCode, NakedP
         int columnNumber = 0;
         String sourceURL = sourceCode.url().string();
         JSC::Strong<JSC::Unknown> error;
-        if (m_workerGlobalScope->sanitizeScriptError(errorMessage, lineNumber, columnNumber, sourceURL, error, sourceCode.cachedScript()))
+        if (m_workerGlobalScope->sanitizeScriptError(errorMessage, lineNumber, columnNumber, sourceURL, error, sourceCode.cachedScript())) {
+            if (returnedExceptionMessage)
+                *returnedExceptionMessage = errorMessage;
             returnedException = JSC::Exception::create(vm, createError(exec, errorMessage.impl()));
+        }
     }
 }
 
index ed0c213..38502a9 100644 (file)
@@ -39,70 +39,70 @@ class VM;
 
 namespace WebCore {
 
-    class JSWorkerGlobalScope;
-    class ScriptSourceCode;
-    class WorkerConsoleClient;
-    class WorkerGlobalScope;
-
-    class WorkerScriptController {
-        WTF_MAKE_NONCOPYABLE(WorkerScriptController); WTF_MAKE_FAST_ALLOCATED;
-    public:
-        WorkerScriptController(WorkerGlobalScope*);
-        ~WorkerScriptController();
-
-        JSWorkerGlobalScope* workerGlobalScopeWrapper()
-        {
-            initScriptIfNeeded();
-            return m_workerGlobalScopeWrapper.get();
-        }
-
-        void evaluate(const ScriptSourceCode&);
-        void evaluate(const ScriptSourceCode&, NakedPtr<JSC::Exception>& returnedException);
-
-        void setException(JSC::Exception*);
-
-        // Async request to terminate a JS run execution. Eventually causes termination
-        // exception raised during JS execution, if the worker thread happens to run JS.
-        // After JS execution was terminated in this way, the Worker thread has to use
-        // forbidExecution()/isExecutionForbidden() to guard against reentry into JS.
-        // Can be called from any thread.
-        void scheduleExecutionTermination();
-        bool isTerminatingExecution() const;
-
-        // Called on Worker thread when JS exits with termination exception caused by forbidExecution() request,
-        // or by Worker thread termination code to prevent future entry into JS.
-        void forbidExecution();
-        bool isExecutionForbidden() const;
-
-        void disableEval(const String& errorMessage);
-        void disableWebAssembly(const String& errorMessage);
-
-        JSC::VM& vm() { return *m_vm; }
-        
-        void releaseHeapAccess();
-        void acquireHeapAccess();
-
-        void addTimerSetNotification(JSC::JSRunLoopTimer::TimerNotificationCallback);
-        void removeTimerSetNotification(JSC::JSRunLoopTimer::TimerNotificationCallback);
-
-        void attachDebugger(JSC::Debugger*);
-        void detachDebugger(JSC::Debugger*);
-
-    private:
-        void initScriptIfNeeded()
-        {
-            if (!m_workerGlobalScopeWrapper)
-                initScript();
-        }
-        void initScript();
-
-        RefPtr<JSC::VM> m_vm;
-        WorkerGlobalScope* m_workerGlobalScope;
-        JSC::Strong<JSWorkerGlobalScope> m_workerGlobalScopeWrapper;
-        std::unique_ptr<WorkerConsoleClient> m_consoleClient;
-        bool m_executionForbidden { false };
-        bool m_isTerminatingExecution { false };
-        mutable Lock m_scheduledTerminationMutex;
-    };
+class JSWorkerGlobalScope;
+class ScriptSourceCode;
+class WorkerConsoleClient;
+class WorkerGlobalScope;
+
+class WorkerScriptController {
+    WTF_MAKE_NONCOPYABLE(WorkerScriptController); WTF_MAKE_FAST_ALLOCATED;
+public:
+    WorkerScriptController(WorkerGlobalScope*);
+    ~WorkerScriptController();
+
+    JSWorkerGlobalScope* workerGlobalScopeWrapper()
+    {
+        initScriptIfNeeded();
+        return m_workerGlobalScopeWrapper.get();
+    }
+
+    void evaluate(const ScriptSourceCode&, String* returnedExceptionMessage = nullptr);
+    void evaluate(const ScriptSourceCode&, NakedPtr<JSC::Exception>& returnedException, String* returnedExceptionMessage = nullptr);
+
+    void setException(JSC::Exception*);
+
+    // Async request to terminate a JS run execution. Eventually causes termination
+    // exception raised during JS execution, if the worker thread happens to run JS.
+    // After JS execution was terminated in this way, the Worker thread has to use
+    // forbidExecution()/isExecutionForbidden() to guard against reentry into JS.
+    // Can be called from any thread.
+    void scheduleExecutionTermination();
+    bool isTerminatingExecution() const;
+
+    // Called on Worker thread when JS exits with termination exception caused by forbidExecution() request,
+    // or by Worker thread termination code to prevent future entry into JS.
+    void forbidExecution();
+    bool isExecutionForbidden() const;
+
+    void disableEval(const String& errorMessage);
+    void disableWebAssembly(const String& errorMessage);
+
+    JSC::VM& vm() { return *m_vm; }
+    
+    void releaseHeapAccess();
+    void acquireHeapAccess();
+
+    void addTimerSetNotification(JSC::JSRunLoopTimer::TimerNotificationCallback);
+    void removeTimerSetNotification(JSC::JSRunLoopTimer::TimerNotificationCallback);
+
+    void attachDebugger(JSC::Debugger*);
+    void detachDebugger(JSC::Debugger*);
+
+private:
+    void initScriptIfNeeded()
+    {
+        if (!m_workerGlobalScopeWrapper)
+            initScript();
+    }
+    void initScript();
+
+    RefPtr<JSC::VM> m_vm;
+    WorkerGlobalScope* m_workerGlobalScope;
+    JSC::Strong<JSWorkerGlobalScope> m_workerGlobalScopeWrapper;
+    std::unique_ptr<WorkerConsoleClient> m_consoleClient;
+    bool m_executionForbidden { false };
+    bool m_isTerminatingExecution { false };
+    mutable Lock m_scheduledTerminationMutex;
+};
 
 } // namespace WebCore
index 0b6c949..684196c 100644 (file)
@@ -91,7 +91,7 @@ void WorkerMessagingProxy::startWorkerGlobalScope(const URL& scriptURL, const St
     auto thread = DedicatedWorkerThread::create(scriptURL, identifier, userAgent, sourceCode, *this, *this, *this, startMode, contentSecurityPolicyResponseHeaders, shouldBypassMainWorldContentSecurityPolicy, document.topOrigin(), timeOrigin, proxy, socketProvider, runtimeFlags, sessionID);
 
     workerThreadCreated(thread.get());
-    thread->start();
+    thread->start(nullptr);
 
     m_inspectorProxy->workerStarted(m_scriptExecutionContext.get(), thread.ptr(), scriptURL);
 }
index 18f8a10..f0797d5 100644 (file)
@@ -128,7 +128,7 @@ WorkerThread::~WorkerThread()
     workerThreads().remove(this);
 }
 
-bool WorkerThread::start()
+bool WorkerThread::start(WTF::Function<void(const String&)>&& evaluateCallback)
 {
     // Mutex protection is necessary to ensure that m_thread is initialized when the thread starts.
     LockHolder lock(m_threadCreationAndWorkerGlobalScopeMutex);
@@ -136,6 +136,8 @@ bool WorkerThread::start()
     if (m_thread)
         return true;
 
+    m_evaluateCallback = WTFMove(evaluateCallback);
+
     m_thread = Thread::create("WebCore: Worker", [this] {
         workerThread();
     });
@@ -181,7 +183,14 @@ void WorkerThread::workerThread()
             scriptController->forbidExecution();
     }
 
-    scriptController->evaluate(ScriptSourceCode(m_startupData->m_sourceCode, m_startupData->m_scriptURL));
+    String exceptionMessage;
+    scriptController->evaluate(ScriptSourceCode(m_startupData->m_sourceCode, m_startupData->m_scriptURL), &exceptionMessage);
+    
+    RunLoop::main().dispatch([evaluateCallback = WTFMove(m_evaluateCallback), message = exceptionMessage.isolatedCopy()] {
+        if (evaluateCallback)
+            evaluateCallback(message);
+    });
+    
     // Free the startup data to cause its member variable deref's happen on the worker's thread (since
     // all ref/derefs of these objects are happening on the thread at this point). Note that
     // WorkerThread::~WorkerThread happens on a different thread where it was created.
index 656f7cb..8084623 100644 (file)
@@ -29,6 +29,7 @@
 #include <memory>
 #include <runtime/RuntimeFlags.h>
 #include <wtf/Forward.h>
+#include <wtf/Function.h>
 #include <wtf/RefCounted.h>
 
 namespace PAL {
@@ -62,7 +63,7 @@ class WorkerThread : public RefCounted<WorkerThread> {
 public:
     virtual ~WorkerThread();
 
-    WEBCORE_EXPORT bool start();
+    WEBCORE_EXPORT bool start(WTF::Function<void(const String&)>&& evaluateCallback);
     void stop();
 
     ThreadIdentifier threadID() const { return m_thread ? m_thread->id() : 0; }
@@ -114,6 +115,8 @@ private:
     Lock m_threadCreationAndWorkerGlobalScopeMutex;
 
     std::unique_ptr<WorkerThreadStartupData> m_startupData;
+    
+    WTF::Function<void(const String&)> m_evaluateCallback;
 
 #if ENABLE(NOTIFICATIONS)
     NotificationClient* m_notificationClient { nullptr };
index 946b3fd..2d19e28 100644 (file)
@@ -47,11 +47,16 @@ auto SWContextManager::connection() const -> Connection*
     return m_connection.get();
 }
 
-void SWContextManager::registerServiceWorkerThread(Ref<ServiceWorkerThreadProxy>&& serviceWorkerThreadProxy)
+void SWContextManager::registerServiceWorkerThreadForUpdate(Ref<ServiceWorkerThreadProxy>&& serviceWorkerThreadProxy)
 {
     auto serviceWorkerIdentifier = serviceWorkerThreadProxy->identifier();
+    auto* threadProxy = serviceWorkerThreadProxy.ptr();
     auto result = m_workerMap.add(serviceWorkerIdentifier, WTFMove(serviceWorkerThreadProxy));
     ASSERT_UNUSED(result, result.isNewEntry);
+    
+    threadProxy->thread().start([serviceWorkerIdentifier](const String& exceptionMessage) {
+        SWContextManager::singleton().connection()->serviceWorkerStartedWithMessage(serviceWorkerIdentifier, exceptionMessage);
+    });
 }
 
 ServiceWorkerThreadProxy* SWContextManager::serviceWorkerThreadProxy(uint64_t serviceWorkerIdentifier) const
index 9d62400..960f47d 100644 (file)
@@ -45,12 +45,13 @@ public:
         virtual ~Connection() { }
 
         virtual void postMessageToServiceWorkerClient(const ServiceWorkerClientIdentifier& destinationIdentifier, Ref<SerializedScriptValue>&& message, uint64_t sourceServiceWorkerIdentifier, const String& sourceOrigin) = 0;
+        virtual void serviceWorkerStartedWithMessage(uint64_t serviceWorkerIdentifier, const String& exceptionMessage) = 0;
     };
 
     WEBCORE_EXPORT void setConnection(std::unique_ptr<Connection>&&);
     WEBCORE_EXPORT Connection* connection() const;
 
-    WEBCORE_EXPORT void registerServiceWorkerThread(Ref<ServiceWorkerThreadProxy>&&);
+    WEBCORE_EXPORT void registerServiceWorkerThreadForUpdate(Ref<ServiceWorkerThreadProxy>&&);
     WEBCORE_EXPORT ServiceWorkerThreadProxy* serviceWorkerThreadProxy(uint64_t) const;
     WEBCORE_EXPORT void postMessageToServiceWorkerGlobalScope(uint64_t destinationServiceWorkerIdentifier, Ref<SerializedScriptValue>&& message, const ServiceWorkerClientIdentifier& sourceIdentifier, const String& sourceOrigin);
 
index 6103324..47795b1 100644 (file)
@@ -57,6 +57,9 @@ public:
     WEBCORE_EXPORT void postFetchTask(Ref<ServiceWorkerFetch::Client>&&, ResourceRequest&&, FetchOptions&&);
     WEBCORE_EXPORT void postMessageToServiceWorkerGlobalScope(Ref<SerializedScriptValue>&&, std::unique_ptr<MessagePortChannelArray>&&, const ServiceWorkerClientIdentifier& sourceIdentifier, const String& sourceOrigin);
 
+    uint64_t serverConnectionIdentifier() const { return m_serverConnectionIdentifier; }
+    const ServiceWorkerContextData& contextData() const { return m_data; }
+
 protected:
     Ref<WorkerGlobalScope> createWorkerGlobalScope(const URL&, const String& identifier, const String& userAgent, const ContentSecurityPolicyResponseHeaders&, bool shouldBypassMainWorldContentSecurityPolicy, Ref<SecurityOrigin>&& topOrigin, MonotonicTime timeOrigin, PAL::SessionID) final;
     void runEventLoop() override;
index 81025f0..75f4884 100644 (file)
@@ -38,9 +38,7 @@ namespace WebCore {
 
 Ref<ServiceWorkerThreadProxy> ServiceWorkerThreadProxy::create(PageConfiguration&& pageConfiguration, uint64_t serverConnectionIdentifier, const ServiceWorkerContextData& data, PAL::SessionID sessionID, CacheStorageProvider& cacheStorageProvider)
 {
-    auto serviceWorker = adoptRef(*new ServiceWorkerThreadProxy { WTFMove(pageConfiguration), serverConnectionIdentifier, data, sessionID, cacheStorageProvider });
-    serviceWorker->m_serviceWorkerThread->start();
-    return serviceWorker;
+    return adoptRef(*new ServiceWorkerThreadProxy { WTFMove(pageConfiguration), serverConnectionIdentifier, data, sessionID, cacheStorageProvider });
 }
 
 static inline UniqueRef<Page> createPageForServiceWorker(PageConfiguration&& configuration, const URL& url)
@@ -61,7 +59,6 @@ ServiceWorkerThreadProxy::ServiceWorkerThreadProxy(PageConfiguration&& pageConfi
     , m_cacheStorageProvider(cacheStorageProvider)
     , m_sessionID(sessionID)
 {
-    m_serviceWorkerThread->start();
 }
 
 bool ServiceWorkerThreadProxy::postTaskForModeToWorkerGlobalScope(ScriptExecutionContext::Task&& task, const String& mode)
index fba6503..ef27875 100644 (file)
@@ -179,14 +179,14 @@ void SWServer::scriptContextStarted(Connection& connection, const ServiceWorkerR
         registration->scriptContextStarted(connection, identifier, workerID);
 }
 
-Ref<SWServerWorker> SWServer::createWorker(Connection& connection, const ServiceWorkerRegistrationKey& registrationKey, const URL& url, const String& script, WorkerType type)
+Ref<SWServerWorker> SWServer::updateWorker(Connection& connection, const ServiceWorkerRegistrationKey& registrationKey, const URL& url, const String& script, WorkerType type)
 {
     String workerID = createCanonicalUUIDString();
     
     auto result = m_workersByID.add(workerID, SWServerWorker::create(registrationKey, url, script, type, workerID));
     ASSERT(result.isNewEntry);
     
-    connection.startServiceWorkerContext({ registrationKey, workerID, script, url });
+    connection.updateServiceWorkerContext({ registrationKey, workerID, script, url });
     
     return result.iterator->value.get();
 }
index c9669da..1d68597 100644 (file)
@@ -72,7 +72,7 @@ public:
         virtual void startScriptFetchInClient(uint64_t jobIdentifier) = 0;
 
         // Messages to the SW host WebProcess
-        virtual void startServiceWorkerContext(const ServiceWorkerContextData&) = 0;
+        virtual void updateServiceWorkerContext(const ServiceWorkerContextData&) = 0;
 
         SWServer& m_server;
     };
@@ -89,7 +89,7 @@ public:
     void postTask(CrossThreadTask&&);
     void postTaskReply(CrossThreadTask&&);
 
-    Ref<SWServerWorker> createWorker(Connection&, const ServiceWorkerRegistrationKey&, const URL&, const String& script, WorkerType);
+    Ref<SWServerWorker> updateWorker(Connection&, const ServiceWorkerRegistrationKey&, const URL&, const String& script, WorkerType);
     
 private:
     void registerConnection(Connection&);
index 3436b6f..aacad4d 100644 (file)
@@ -84,14 +84,18 @@ void SWServerRegistration::scriptFetchFinished(SWServer::Connection& connection,
     // then resolve and finish the job without doing anything further.
 
     // FIXME: Support the proper worker type (classic vs module)
-    m_server.createWorker(connection, m_registrationKey, m_currentJob->scriptURL, result.script, WorkerType::Classic);
+    m_server.updateWorker(connection, m_registrationKey, m_currentJob->scriptURL, result.script, WorkerType::Classic);
 }
 
 void SWServerRegistration::scriptContextFailedToStart(SWServer::Connection&, const String& workerID, const String& message)
 {
-    UNUSED_PARAM(workerID);
+    // FIXME: Install has failed. Run the install failed substeps
+    // Run the Update Worker State algorithm passing registration’s installing worker and redundant as the arguments.
+    // Run the Update Registration State algorithm passing registration, "installing" and null as the arguments.
+    // If newestWorker is null, invoke Clear Registration algorithm passing registration as its argument.
 
-    rejectCurrentJob(ExceptionData { UnknownError, message });
+    UNUSED_PARAM(workerID);
+    UNUSED_PARAM(message);
 }
 
 void SWServerRegistration::scriptContextStarted(SWServer::Connection&, uint64_t identifier, const String& workerID)
index 4c1a206..f30952d 100644 (file)
@@ -1,3 +1,23 @@
+2017-11-01  Brady Eidson  <beidson@apple.com>
+
+        Plumbing for handling SW scripts failing to evaluate
+        https://bugs.webkit.org/show_bug.cgi?id=178926
+
+        Reviewed by Chris Dumez.
+
+        * StorageProcess/ServiceWorker/WebSWServerConnection.cpp:
+        (WebKit::WebSWServerConnection::updateServiceWorkerContext):
+        (WebKit::WebSWServerConnection::setContextConnection):
+        (WebKit::WebSWServerConnection::startServiceWorkerContext): Deleted.
+        * StorageProcess/ServiceWorker/WebSWServerConnection.h:
+
+        * WebProcess/Storage/WebSWContextManagerConnection.cpp:
+        (WebKit::WebSWContextManagerConnection::updateServiceWorker):
+        (WebKit::WebSWContextManagerConnection::serviceWorkerStartedWithMessage):
+        (WebKit::WebSWContextManagerConnection::startServiceWorker): Deleted.
+        * WebProcess/Storage/WebSWContextManagerConnection.h:
+        * WebProcess/Storage/WebSWContextManagerConnection.messages.in:
+
 2017-11-01  Frederic Wang  <fwang@igalia.com>
 
         Make iOS Find UI reveal matches in scrollable elements
index 006436f..f7bca3d 100644 (file)
@@ -96,9 +96,9 @@ void WebSWServerConnection::startScriptFetchInClient(uint64_t jobIdentifier)
     send(Messages::WebSWClientConnection::StartScriptFetchForServer(jobIdentifier));
 }
 
-void WebSWServerConnection::startServiceWorkerContext(const ServiceWorkerContextData& data)
+void WebSWServerConnection::updateServiceWorkerContext(const ServiceWorkerContextData& data)
 {
-    if (sendToContextProcess(Messages::WebSWContextManagerConnection::StartServiceWorker(identifier(), data)))
+    if (sendToContextProcess(Messages::WebSWContextManagerConnection::UpdateServiceWorker(identifier(), data)))
         return;
 
     m_pendingContextDatas.append(data);
@@ -157,9 +157,9 @@ void WebSWServerConnection::setContextConnection(IPC::Connection* connection)
 {
     m_contextConnection = connection;
 
-    // We can now start any pending service worker contexts.
+    // We can now start any pending service worker updates.
     for (auto& pendingContextData : m_pendingContextDatas)
-        startServiceWorkerContext(pendingContextData);
+        updateServiceWorkerContext(pendingContextData);
     
     m_pendingContextDatas.clear();
 }
index 60397e9..3102e16 100644 (file)
@@ -71,7 +71,7 @@ private:
     void postMessageToServiceWorkerGlobalScope(uint64_t destinationServiceWorkerIdentifier, const IPC::DataReference& message, uint64_t sourceScriptExecutionContextIdentifier, const String& sourceOrigin);
 
     // Messages to the SW context WebProcess
-    void startServiceWorkerContext(const WebCore::ServiceWorkerContextData&) final;
+    void updateServiceWorkerContext(const WebCore::ServiceWorkerContextData&) final;
 
     IPC::Connection* messageSenderConnection() final { return m_contentConnection.ptr(); }
     uint64_t messageSenderDestinationID() final { return identifier(); }
index c7e1499..d719aa5 100644 (file)
@@ -95,7 +95,7 @@ void WebSWContextManagerConnection::updatePreferences(const WebPreferencesStore&
     RuntimeEnabledFeatures::sharedFeatures().setFetchAPIEnabled(store.getBoolValueForKey(WebPreferencesKey::fetchAPIEnabledKey()));
 }
 
-void WebSWContextManagerConnection::startServiceWorker(uint64_t serverConnectionIdentifier, const ServiceWorkerContextData& data)
+void WebSWContextManagerConnection::updateServiceWorker(uint64_t serverConnectionIdentifier, const ServiceWorkerContextData& data)
 {
     // FIXME: Provide a sensical session ID.
     auto sessionID = PAL::SessionID::defaultSessionID();
@@ -107,16 +107,29 @@ void WebSWContextManagerConnection::startServiceWorker(uint64_t serverConnection
         WebProcess::singleton().cacheStorageProvider()
     };
     fillWithEmptyClients(pageConfiguration);
+
+    // FIXME: This method should be moved directly to WebCore::SWContextManager::Connection
+    // If it weren't for ServiceWorkerFrameLoaderClient's dependence on WebDocumentLoader, this could already happen.
     auto frameLoaderClient = std::make_unique<ServiceWorkerFrameLoaderClient>(sessionID, m_pageID, ++m_previousServiceWorkerID);
     pageConfiguration.loaderClientForMainFrame = frameLoaderClient.release();
 
     auto serviceWorkerThreadProxy = ServiceWorkerThreadProxy::create(WTFMove(pageConfiguration), serverConnectionIdentifier, data, sessionID, WebProcess::singleton().cacheStorageProvider());
-    auto serviceWorkerIdentifier = serviceWorkerThreadProxy->identifier();
-    SWContextManager::singleton().registerServiceWorkerThread(WTFMove(serviceWorkerThreadProxy));
+    SWContextManager::singleton().registerServiceWorkerThreadForUpdate(WTFMove(serviceWorkerThreadProxy));
 
-    LOG(ServiceWorker, "Context process PID: %i started worker thread %s\n", getpid(), data.workerID.utf8().data());
+    LOG(ServiceWorker, "Context process PID: %i created worker thread %s\n", getpid(), data.workerID.utf8().data());
+}
 
-    m_connectionToStorageProcess->send(Messages::StorageProcess::ServiceWorkerContextStarted(serverConnectionIdentifier, data.registrationKey, serviceWorkerIdentifier, data.workerID), 0);
+void WebSWContextManagerConnection::serviceWorkerStartedWithMessage(uint64_t serviceWorkerIdentifier, const String& exceptionMessage)
+{
+    auto* threadProxy = SWContextManager::singleton().serviceWorkerThreadProxy(serviceWorkerIdentifier);
+    ASSERT(threadProxy);
+    
+    auto& data = threadProxy->thread().contextData();
+    
+    if (exceptionMessage.isEmpty())
+        m_connectionToStorageProcess->send(Messages::StorageProcess::ServiceWorkerContextStarted(threadProxy->thread().serverConnectionIdentifier(), data.registrationKey, serviceWorkerIdentifier, data.workerID), 0);
+    else
+        m_connectionToStorageProcess->send(Messages::StorageProcess::ServiceWorkerContextFailedToStart(threadProxy->thread().serverConnectionIdentifier(), data.registrationKey, data.workerID, exceptionMessage), 0);
 }
 
 void WebSWContextManagerConnection::startFetch(uint64_t serverConnectionIdentifier, uint64_t fetchIdentifier, uint64_t serviceWorkerIdentifier, ResourceRequest&& request, FetchOptions&& options)
index 2da096c..2310d93 100644 (file)
@@ -54,7 +54,8 @@ private:
     void postMessageToServiceWorkerClient(const WebCore::ServiceWorkerClientIdentifier& destinationIdentifier, Ref<WebCore::SerializedScriptValue>&& message, uint64_t sourceServiceWorkerIdentifier, const String& sourceOrigin) final;
 
     // IPC messages.
-    void startServiceWorker(uint64_t serverConnectionIdentifier, const WebCore::ServiceWorkerContextData&);
+    void serviceWorkerStartedWithMessage(uint64_t serviceWorkerIdentifier, const String& exceptionMessage) final;
+    void updateServiceWorker(uint64_t serverConnectionIdentifier, const WebCore::ServiceWorkerContextData&);
     void startFetch(uint64_t serverConnectionIdentifier, uint64_t fetchIdentifier, uint64_t serviceWorkerIdentifier, WebCore::ResourceRequest&&, WebCore::FetchOptions&&);
     void postMessageToServiceWorkerGlobalScope(uint64_t destinationServiceWorkerIdentifier, const IPC::DataReference& message, const WebCore::ServiceWorkerClientIdentifier& sourceIdentifier, const String& sourceOrigin);
 
index 94b8e03..e032cd7 100644 (file)
@@ -23,7 +23,7 @@
 #if ENABLE(SERVICE_WORKER)
 
 messages -> WebSWContextManagerConnection {
-    StartServiceWorker(uint64_t serverConnectionIdentifier, struct WebCore::ServiceWorkerContextData contextData)
+    UpdateServiceWorker(uint64_t serverConnectionIdentifier, struct WebCore::ServiceWorkerContextData contextData)
     StartFetch(uint64_t serverConnectionIdentifier, uint64_t fetchIdentifier, uint64_t serviceWorkerIdentifier, WebCore::ResourceRequest request, struct WebCore::FetchOptions options)
     PostMessageToServiceWorkerGlobalScope(uint64_t destinationServiceWorkerIdentifier, IPC::DataReference message, struct WebCore::ServiceWorkerClientIdentifier sourceIdentifier, String sourceOrigin)
 }