shared worker: Use explicit ownership for WebSharedWorker.

This patch removes explicit delete and raw pointers and makes
ownership more clear by having WebSharedWorkerClient own
WebSharedWorker.

Change-Id: Ifd352b3effd9f2b46c9e61e792237ed3b6c2024a
Reviewed-on: https://chromium-review.googlesource.com/994893
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548036}

content/renderer/shared_worker/embedded_shared_worker_stub.cc

@@ -156,7 +156,8 @@ EmbeddedSharedWorkerStub::EmbeddedSharedWorkerStub(
 }
 
 EmbeddedSharedWorkerStub::~EmbeddedSharedWorkerStub() {
-  DCHECK(!impl_);
+  // Destruction closes our connection to the host, triggering the host to
+  // cleanup and notify clients of this worker going away.
 }
 
 void EmbeddedSharedWorkerStub::WorkerReadyForInspection() {
@@ -175,7 +176,7 @@ void EmbeddedSharedWorkerStub::WorkerScriptLoaded() {
 void EmbeddedSharedWorkerStub::WorkerScriptLoadFailed() {
   host_->OnScriptLoadFailed();
   pending_channels_.clear();
-  Shutdown();
+  delete this;
 }
 
 void EmbeddedSharedWorkerStub::CountFeature(blink::mojom::WebFeature feature) {
@@ -187,7 +188,7 @@ void EmbeddedSharedWorkerStub::WorkerContextClosed() {
 }
 
 void EmbeddedSharedWorkerStub::WorkerContextDestroyed() {
-  Shutdown();
+  delete this;
 }
 
 void EmbeddedSharedWorkerStub::SelectAppCacheID(long long app_cache_id) {
@@ -290,16 +291,6 @@ EmbeddedSharedWorkerStub::CreateWorkerFetchContext(
   return std::move(worker_fetch_context);
 }
 
-void EmbeddedSharedWorkerStub::Shutdown() {
-  // WebSharedWorker must be already deleted in the blink side
-  // when this is called.
-  impl_ = nullptr;
-
-  // This closes our connection to the host, triggering the host to cleanup and
-  // notify clients of this worker going away.
-  delete this;
-}
-
 void EmbeddedSharedWorkerStub::ConnectToChannel(
     int connection_request_id,
     blink::MessagePortChannel channel) {

content/renderer/shared_worker/embedded_shared_worker_stub.h

@@ -42,14 +42,13 @@ class WebApplicationCacheHostImpl;
 
 // A stub class to receive IPC from browser process and talk to
 // blink::WebSharedWorker. Implements blink::WebSharedWorkerClient.
-// This class is self-destruct (no one explicitly owns this), and
-// deletes itself (via private Shutdown() method) when either one of
-// following methods is called by blink::WebSharedWorker:
-// - workerScriptLoadFailed() or
-// - workerContextDestroyed()
+// This class is self-destructed (no one explicitly owns this). It deletes
+// itself when either one of following methods is called by
+// blink::WebSharedWorker:
+// - WorkerScriptLoadFailed() or
+// - WorkerContextDestroyed()
 //
-// In either case the corresponding blink::WebSharedWorker also deletes
-// itself.
+// This class owns blink::WebSharedWorker.
 class EmbeddedSharedWorkerStub : public blink::WebSharedWorkerClient,
                                  public mojom::SharedWorker {
  public:
@@ -80,8 +79,6 @@ class EmbeddedSharedWorkerStub : public blink::WebSharedWorkerClient,
       blink::WebServiceWorkerNetworkProvider*) override;
 
  private:
-  void Shutdown();
-
   // WebSharedWorker will own |channel|.
   void ConnectToChannel(int connection_request_id,
                         blink::MessagePortChannel channel);
@@ -98,7 +95,7 @@ class EmbeddedSharedWorkerStub : public blink::WebSharedWorkerClient,
   const std::string name_;
   bool running_ = false;
   GURL url_;
-  blink::WebSharedWorker* impl_ = nullptr;
+  std::unique_ptr<blink::WebSharedWorker> impl_;
 
   using PendingChannel =
       std::pair<int /* connection_request_id */, blink::MessagePortChannel>;
@@ -106,6 +103,7 @@ class EmbeddedSharedWorkerStub : public blink::WebSharedWorkerClient,
 
   ScopedChildProcessReference process_ref_;
   WebApplicationCacheHostImpl* app_cache_host_ = nullptr;  // Not owned.
+
   DISALLOW_COPY_AND_ASSIGN(EmbeddedSharedWorkerStub);
 };
 

third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp

@@ -91,14 +91,14 @@ void WebSharedWorkerImpl::TerminateWorkerThread() {
   asked_to_terminate_ = true;
   if (shadow_page_ && !shadow_page_->WasInitialized()) {
     client_->WorkerScriptLoadFailed();
-    delete this;
+    // |this| is deleted at this point.
     return;
   }
   if (main_script_loader_) {
     main_script_loader_->Cancel();
     main_script_loader_ = nullptr;
     client_->WorkerScriptLoadFailed();
-    delete this;
+    // |this| is deleted at this point.
     return;
   }
   if (worker_thread_)
@@ -175,8 +175,7 @@ void WebSharedWorkerImpl::DidCloseWorkerGlobalScope() {
 void WebSharedWorkerImpl::DidTerminateWorkerThread() {
   DCHECK(IsMainThread());
   client_->WorkerContextDestroyed();
-  // The lifetime of this proxy is controlled by the worker context.
-  delete this;
+  // |this| is deleted at this point.
 }
 
 void WebSharedWorkerImpl::Connect(MessagePortChannel web_channel) {
@@ -252,10 +251,7 @@ void WebSharedWorkerImpl::OnScriptLoaderFinished() {
   if (main_script_loader_->Failed()) {
     main_script_loader_->Cancel();
     client_->WorkerScriptLoadFailed();
-
-    // The SharedWorker was unable to load the initial script, so
-    // shut it down right here.
-    delete this;
+    // |this| is deleted at this point.
     return;
   }
 
@@ -362,8 +358,9 @@ void WebSharedWorkerImpl::BindDevToolsAgent(
       std::move(devtools_agent_request)));
 }
 
-WebSharedWorker* WebSharedWorker::Create(WebSharedWorkerClient* client) {
-  return new WebSharedWorkerImpl(client);
+std::unique_ptr<WebSharedWorker> WebSharedWorker::Create(
+    WebSharedWorkerClient* client) {
+  return base::WrapUnique(new WebSharedWorkerImpl(client));
 }
 
 }  // namespace blink

third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.h

@@ -34,6 +34,7 @@
 #include "public/web/WebSharedWorker.h"
 
 #include <memory>
+
 #include "base/memory/scoped_refptr.h"
 #include "core/CoreExport.h"
 #include "core/exported/WorkerShadowPage.h"
@@ -61,10 +62,13 @@ class WorkerInspectorProxy;
 // implementation. This is basically accessed on the main thread, but some
 // methods must be called from a worker thread. Such methods are suffixed with
 // *OnWorkerThread or have header comments.
+//
+// Owned by WebSharedWorkerClient.
 class CORE_EXPORT WebSharedWorkerImpl final : public WebSharedWorker,
                                               public WorkerShadowPage::Client {
  public:
   explicit WebSharedWorkerImpl(WebSharedWorkerClient*);
+  ~WebSharedWorkerImpl() override;
 
   // WorkerShadowPage::Client overrides.
   std::unique_ptr<WebApplicationCacheHost> CreateApplicationCacheHost(
@@ -99,8 +103,6 @@ class CORE_EXPORT WebSharedWorkerImpl final : public WebSharedWorker,
   void DidTerminateWorkerThread();
 
  private:
-  ~WebSharedWorkerImpl() override;
-
   WorkerThread* GetWorkerThread() { return worker_thread_.get(); }
 
   // Shuts down the worker thread.
@@ -124,6 +126,7 @@ class CORE_EXPORT WebSharedWorkerImpl final : public WebSharedWorker,
   std::unique_ptr<WorkerThread> worker_thread_;
   mojom::blink::WorkerContentSettingsProxyPtrInfo content_settings_info_;
 
+  // |client_| owns |this|.
   WebSharedWorkerClient* client_;
 
   bool asked_to_terminate_ = false;

third_party/WebKit/public/web/WebSharedWorker.h

@@ -31,6 +31,8 @@
 #ifndef WebSharedWorker_h
 #define WebSharedWorker_h
 
+#include <memory>
+
 #include "base/unguessable_token.h"
 #include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"
 #include "mojo/public/cpp/system/message_pipe.h"
@@ -48,10 +50,11 @@ class WebURL;
 // This is the interface to a SharedWorker thread.
 class BLINK_EXPORT WebSharedWorker {
  public:
+  virtual ~WebSharedWorker() {}
+
   // Instantiate a WebSharedWorker that interacts with the shared worker.
-  // WebSharedWorkerClient given here must outlive or have the identical
-  // lifetime as this instance.
-  static WebSharedWorker* Create(WebSharedWorkerClient*);
+  // WebSharedWorkerClient given here should own this instance.
+  static std::unique_ptr<WebSharedWorker> Create(WebSharedWorkerClient*);
 
   virtual void StartWorkerContext(
       const WebURL& script_url,

third_party/WebKit/public/web/WebSharedWorkerClient.h

@@ -45,9 +45,10 @@ class WebServiceWorkerNetworkProvider;
 // Provides an interface back to the in-page script object for a worker.
 // All functions are expected to be called back on the thread that created
 // the Worker object, unless noted.
-// An instance of this class must outlive or must have the identical lifetime
-// as WebSharedWorker (i.e. must be kept alive until workerScriptLoadFailed()
-// or workerContextDestroyed() is called).
+//
+// An instance of this class must outlive WebSharedWorker (i.e. must be kept
+// alive until WorkerScriptLoadFailed() or WorkerContextDestroyed() is
+// called).
 class WebSharedWorkerClient {
  public:
   virtual void CountFeature(mojom::WebFeature) = 0;