ServiceWorkerInstalledScriptsManager handles all installed scripts

As a step of removing SWReadFromCacheJob, this patch adds a path to get the same
installed scripts multiple times. WebServiceWorkerInstalledScriptsManager asks
the browser if the script has already been taken, and the browser side
counterpart (SWInstalledScriptsSender) sends the script after streaming all of
scripts. This is not optimal for sites which importing the same script multiple
times, but it simplifies the code.

Bug: 756312
Change-Id: I8d8c03fe8488367b5aa1b8d2c02e13a5f25e676e
Reviewed-on: https://chromium-review.googlesource.com/697165
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509712}

content/browser/service_worker/embedded_worker_instance_unittest.cc

@@ -207,6 +207,8 @@ class EmbeddedWorkerInstanceTest : public testing::Test,
     auto info = mojom::ServiceWorkerInstalledScriptsInfo::New();
     info->manager_request =
         mojo::MakeRequest(&installed_scripts_managers_.back());
+    installed_scripts_manager_host_requests_.push_back(
+        mojo::MakeRequest(&info->manager_host_ptr));
     return info;
   }
 
@@ -230,6 +232,8 @@ class EmbeddedWorkerInstanceTest : public testing::Test,
   std::vector<mojom::ControllerServiceWorkerPtr> controllers_;
   std::vector<mojom::ServiceWorkerInstalledScriptsManagerPtr>
       installed_scripts_managers_;
+  std::vector<mojom::ServiceWorkerInstalledScriptsManagerHostRequest>
+      installed_scripts_manager_host_requests_;
   std::vector<std::unique_ptr<ProviderHostEndpoints>> provider_host_endpoints_;
 
   TestBrowserThreadBundle thread_bundle_;

content/browser/service_worker/service_worker_installed_scripts_sender.h

@@ -5,7 +5,10 @@
 #ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_INSTALLED_SCRIPTS_SENDER_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_INSTALLED_SCRIPTS_SENDER_H_
 
+#include <queue>
+
 #include "content/common/service_worker/service_worker_installed_scripts_manager.mojom.h"
+#include "mojo/public/cpp/bindings/binding.h"
 
 namespace content {
 
@@ -16,7 +19,18 @@ class ServiceWorkerVersion;
 // scripts from ServiceWorkerStorage to the renderer through Mojo data pipes.
 // ServiceWorkerInstalledScriptsSender is owned by ServiceWorkerVersion. It is
 // created for worker startup and lives as long as the worker is running.
-class CONTENT_EXPORT ServiceWorkerInstalledScriptsSender {
+//
+// SWInstalledScriptsSender has three phases.
+// 1. The sender sends all installed scripts to the renderer without any
+//    requests from the renderer. This initial phase is called "streaming".
+//    |state_| is kSendingScripts. When all installed scripts are sent, moves to
+//    the phase 2.
+// 2. The sender is idle. |state_| is kIdle. If the renderer calls
+//    RequestInstalledScript, moves to the phase 3.
+// 3. The sender sends requested scripts. |state_| is kSendingScripts. When all
+//    the requested scripts are sent, returns to the phase 2.
+class CONTENT_EXPORT ServiceWorkerInstalledScriptsSender
+    : public mojom::ServiceWorkerInstalledScriptsManagerHost {
  public:
   // Do not change the order. This is used for UMA.
   enum class FinishedReason {
@@ -34,7 +48,7 @@ class CONTENT_EXPORT ServiceWorkerInstalledScriptsSender {
   // |owner| must be an installed service worker.
   explicit ServiceWorkerInstalledScriptsSender(ServiceWorkerVersion* owner);
 
-  ~ServiceWorkerInstalledScriptsSender();
+  ~ServiceWorkerInstalledScriptsSender() override;
 
   // Creates a Mojo struct (mojom::ServiceWorkerInstalledScriptsInfo) and sets
   // it with the information to create WebServiceWorkerInstalledScriptsManager
@@ -44,20 +58,28 @@ class CONTENT_EXPORT ServiceWorkerInstalledScriptsSender {
   // Starts sending installed scripts to the worker.
   void Start();
 
-  bool IsFinished() const;
-  FinishedReason finished_reason() const { return finished_reason_; }
+  // Returns the reason for the last time the sender entered the idle state. If
+  // this sender has never reached the idle state, returns kNotFinished.
+  FinishedReason last_finished_reason() const { return last_finished_reason_; }
 
  private:
   class Sender;
 
   enum class State {
     kNotStarted,
-    kSendingMainScript,
-    kSendingImportedScript,
-    kFinished,
+    kSendingScripts,
+    kIdle,
   };
 
-  void StartSendingScript(int64_t resource_id);
+  void StartSendingScript(int64_t resource_id, const GURL& script_url);
+
+  // Stops all tasks even if pending scripts exist and disconnects the pipe to
+  // the renderer. Also, if |reason| indicates failure to read the installed
+  // script from the disk cache (kNoHTTPInfoError or kResponseReaderError), then
+  // |owner_| is doomed via ServiceWorkerRegistration::DeleteVersion().
+  void Abort(FinishedReason reason);
+
+  void UpdateFinishedReasonAndBecomeIdle(FinishedReason reason);
 
   // Called from |running_sender_|.
   void SendScriptInfoToRenderer(
@@ -68,24 +90,27 @@ class CONTENT_EXPORT ServiceWorkerInstalledScriptsSender {
       mojo::ScopedDataPipeConsumerHandle meta_data_handle,
       uint64_t meta_data_size);
   void OnHttpInfoRead(scoped_refptr<HttpResponseInfoIOBuffer> http_info);
-  void OnFinishSendingScript();
-  void OnAbortSendingScript(FinishedReason reason);
+  void OnFinishSendingScript(FinishedReason reason);
 
-  const GURL& CurrentSendingURL();
+  // Implements mojom::ServiceWorkerInstalledScriptsManagerHost.
+  void RequestInstalledScript(const GURL& script_url) override;
 
-  void UpdateState(State state);
-  void Finish(FinishedReason reason);
+  bool IsSendingMainScript() const;
 
   ServiceWorkerVersion* owner_;
   const GURL main_script_url_;
   const int64_t main_script_id_;
+  bool sent_main_script_;
 
+  mojo::Binding<mojom::ServiceWorkerInstalledScriptsManagerHost> binding_;
   mojom::ServiceWorkerInstalledScriptsManagerPtr manager_;
   std::unique_ptr<Sender> running_sender_;
+
   State state_;
-  FinishedReason finished_reason_;
-  std::map<int64_t /* resource_id */, GURL> imported_scripts_;
-  std::map<int64_t /* resource_id */, GURL>::iterator imported_script_iter_;
+  FinishedReason last_finished_reason_;
+
+  GURL current_sending_url_;
+  std::queue<std::pair<int64_t /* resource_id */, GURL>> pending_scripts_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerInstalledScriptsSender);
 };

content/browser/service_worker/service_worker_version.cc

@@ -1768,7 +1768,7 @@ void ServiceWorkerVersion::RecordStartWorkerResult(
 
   if (installed_scripts_sender_) {
     ServiceWorkerMetrics::RecordInstalledScriptsSenderStatus(
-        installed_scripts_sender_->finished_reason());
+        installed_scripts_sender_->last_finished_reason());
   }
   ServiceWorkerMetrics::RecordStartWorkerStatus(status, purpose,
                                                 IsInstalled(prestart_status));

content/common/service_worker/service_worker_installed_scripts_manager.mojom

@@ -10,6 +10,7 @@ import "url/mojo/url.mojom";
 // Contains information about the scripts of an installed service worker.
 struct ServiceWorkerInstalledScriptsInfo {
   ServiceWorkerInstalledScriptsManager& manager_request;
+  ServiceWorkerInstalledScriptsManagerHost manager_host_ptr;
   array<url.mojom.Url> installed_urls;
 };
 
@@ -29,6 +30,14 @@ struct ServiceWorkerScriptInfo {
   uint64 meta_data_size;
 };
 
+// Browser-side interface. The renderer uses this interface to request installed
+// scripts from the browser process.
+interface ServiceWorkerInstalledScriptsManagerHost {
+  // Requests to send the installed scripts over
+  // ServiceWorkerInstalledScriptsManager::TransferInstalledScript.
+  RequestInstalledScript(url.mojom.Url script_url);
+};
+
 // Renderer-side interface. The browser uses this interface to send
 // information about installed scripts to the renderer.
 interface ServiceWorkerInstalledScriptsManager {

content/renderer/service_worker/thread_safe_script_container.cc

@@ -14,7 +14,6 @@ ThreadSafeScriptContainer::ThreadSafeScriptContainer()
 void ThreadSafeScriptContainer::AddOnIOThread(const GURL& url,
                                               std::unique_ptr<Data> data) {
   base::AutoLock lock(lock_);
-  DCHECK(script_data_.find(url) == script_data_.end());
   script_data_[url] = std::move(data);
   if (url == waiting_url_)
     waiting_cv_.Signal();
@@ -26,10 +25,16 @@ ThreadSafeScriptContainer::GetStatusOnWorkerThread(const GURL& url) {
   auto it = script_data_.find(url);
   if (it == script_data_.end())
     return ScriptStatus::kPending;
-  // If the instance is invalid, return |kFailed|.
-  // TODO(shimazu): Keep the status for each entries instead of using IsValid().
-  return (it->second && !it->second->IsValid()) ? ScriptStatus::kFailed
-                                                : ScriptStatus::kSuccess;
+  if (!it->second)
+    return ScriptStatus::kTaken;
+  if (!it->second->IsValid())
+    return ScriptStatus::kFailed;
+  return ScriptStatus::kReceived;
+}
+
+void ThreadSafeScriptContainer::ResetOnWorkerThread(const GURL& url) {
+  base::AutoLock lock(lock_);
+  script_data_.erase(url);
 }
 
 bool ThreadSafeScriptContainer::WaitOnWorkerThread(const GURL& url) {
@@ -57,11 +62,9 @@ bool ThreadSafeScriptContainer::WaitOnWorkerThread(const GURL& url) {
 std::unique_ptr<ThreadSafeScriptContainer::Data>
 ThreadSafeScriptContainer::TakeOnWorkerThread(const GURL& url) {
   base::AutoLock lock(lock_);
-  DCHECK(script_data_.find(url) != script_data_.end())
+  DCHECK(base::ContainsKey(script_data_, url))
       << "Script should be added before calling Take.";
-  auto data = std::move(script_data_[url]);
-  DCHECK(script_data_[url] == nullptr);
-  return data;
+  return std::move(script_data_[url]);
 }
 
 void ThreadSafeScriptContainer::OnAllDataAddedOnIOThread() {

content/renderer/service_worker/thread_safe_script_container.h

@@ -37,25 +37,34 @@ class CONTENT_EXPORT ThreadSafeScriptContainer
   REQUIRE_ADOPTION_FOR_REFCOUNTED_TYPE();
   ThreadSafeScriptContainer();
 
-  enum class ScriptStatus { kSuccess, kFailed, kPending };
+  enum class ScriptStatus {
+    // The script data has been received.
+    kReceived,
+    // The script data has been received but it has already been taken.
+    kTaken,
+    // Receiving the script has failed.
+    kFailed,
+    // The script data has not been received yet.
+    kPending
+  };
 
   // Called on the IO thread.
   void AddOnIOThread(const GURL& url, std::unique_ptr<Data> data);
 
-  // Returns the following values.
-  // - |kSuccess| : the script data has been received.
-  // - |kFailed| : receiving the script has failed.
-  // - |kPending| : the script has not been received yet.
   // Called on the worker thread.
   ScriptStatus GetStatusOnWorkerThread(const GURL& url);
 
+  // Removes the script. After calling this, ScriptStatus for the
+  // script will be kPending.
+  // Called on the worker thread.
+  void ResetOnWorkerThread(const GURL& url);
+
   // Waits until the script is added. The thread is blocked until the script is
   // available or receiving the script fails. Returns false if an error happens
   // and the waiting script won't be available forever.
   // Called on the worker thread.
   bool WaitOnWorkerThread(const GURL& url);
 
-  // Returns nullptr if the script has already been taken.
   // Called on the worker thread.
   std::unique_ptr<Data> TakeOnWorkerThread(const GURL& url);
 

content/renderer/service_worker/thread_safe_script_container_unittest.cc

@@ -130,7 +130,7 @@ class ThreadSafeScriptContainerTest : public testing::Test {
 TEST_F(ThreadSafeScriptContainerTest, WaitExistingKey) {
   const GURL kKey("https://example.com/key");
   {
-    ScriptStatus result = ScriptStatus::kSuccess;
+    ScriptStatus result = ScriptStatus::kReceived;
     GetStatusOnReaderThread(kKey, &result)->Wait();
     EXPECT_EQ(ScriptStatus::kPending, result);
   }
@@ -150,7 +150,7 @@ TEST_F(ThreadSafeScriptContainerTest, WaitExistingKey) {
   {
     ScriptStatus result = ScriptStatus::kFailed;
     GetStatusOnReaderThread(kKey, &result)->Wait();
-    EXPECT_EQ(ScriptStatus::kSuccess, result);
+    EXPECT_EQ(ScriptStatus::kReceived, result);
   }
 
   {
@@ -163,7 +163,7 @@ TEST_F(ThreadSafeScriptContainerTest, WaitExistingKey) {
     ScriptStatus result = ScriptStatus::kFailed;
     GetStatusOnReaderThread(kKey, &result)->Wait();
     // The record of |kKey| should be exist though it's already taken.
-    EXPECT_EQ(ScriptStatus::kSuccess, result);
+    EXPECT_EQ(ScriptStatus::kTaken, result);
   }
 
   {
@@ -197,7 +197,7 @@ TEST_F(ThreadSafeScriptContainerTest, WaitExistingKey) {
 TEST_F(ThreadSafeScriptContainerTest, WaitNonExistingKey) {
   const GURL kKey("https://example.com/key");
   {
-    ScriptStatus result = ScriptStatus::kSuccess;
+    ScriptStatus result = ScriptStatus::kReceived;
     GetStatusOnReaderThread(kKey, &result)->Wait();
     EXPECT_EQ(ScriptStatus::kPending, result);
   }

content/renderer/service_worker/web_service_worker_installed_scripts_manager_impl.cc

@@ -224,25 +224,29 @@ WebServiceWorkerInstalledScriptsManagerImpl::Create(
     mojom::ServiceWorkerInstalledScriptsInfoPtr installed_scripts_info,
     scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) {
   auto script_container = base::MakeRefCounted<ThreadSafeScriptContainer>();
-  std::unique_ptr<blink::WebServiceWorkerInstalledScriptsManager>
-      installed_scripts_manager =
-          base::WrapUnique<WebServiceWorkerInstalledScriptsManagerImpl>(
-              new WebServiceWorkerInstalledScriptsManagerImpl(
-                  std::move(installed_scripts_info->installed_urls),
-                  script_container));
+  std::unique_ptr<blink::WebServiceWorkerInstalledScriptsManager> manager =
+      base::WrapUnique<WebServiceWorkerInstalledScriptsManagerImpl>(
+          new WebServiceWorkerInstalledScriptsManagerImpl(
+              std::move(installed_scripts_info->installed_urls),
+              script_container,
+              std::move(installed_scripts_info->manager_host_ptr)));
   io_task_runner->PostTask(
       FROM_HERE,
       base::BindOnce(&Internal::Create, script_container,
                      std::move(installed_scripts_info->manager_request)));
-  return installed_scripts_manager;
+  return manager;
 }
 
 WebServiceWorkerInstalledScriptsManagerImpl::
     WebServiceWorkerInstalledScriptsManagerImpl(
         std::vector<GURL>&& installed_urls,
-        scoped_refptr<ThreadSafeScriptContainer> script_container)
+        scoped_refptr<ThreadSafeScriptContainer> script_container,
+        mojom::ServiceWorkerInstalledScriptsManagerHostPtr manager_host)
     : installed_urls_(installed_urls.begin(), installed_urls.end()),
-      script_container_(std::move(script_container)) {}
+      script_container_(std::move(script_container)),
+      manager_host_(
+          mojom::ThreadSafeServiceWorkerInstalledScriptsManagerHostPtr::Create(
+              std::move(manager_host))) {}
 
 WebServiceWorkerInstalledScriptsManagerImpl::
     ~WebServiceWorkerInstalledScriptsManagerImpl() = default;
@@ -255,11 +259,24 @@ bool WebServiceWorkerInstalledScriptsManagerImpl::IsScriptInstalled(
 std::unique_ptr<RawScriptData>
 WebServiceWorkerInstalledScriptsManagerImpl::GetRawScriptData(
     const blink::WebURL& script_url) {
+  TRACE_EVENT1("ServiceWorker",
+               "WebServiceWorkerInstalledScriptsManagerImpl::GetRawScriptData",
+               "script_url", script_url.GetString().Utf8());
   if (!IsScriptInstalled(script_url))
     return nullptr;
 
   ThreadSafeScriptContainer::ScriptStatus status =
       script_container_->GetStatusOnWorkerThread(script_url);
+  // If the script has already been taken, request the browser to send the
+  // script.
+  if (status == ThreadSafeScriptContainer::ScriptStatus::kTaken) {
+    script_container_->ResetOnWorkerThread(script_url);
+    (*manager_host_)->RequestInstalledScript(script_url);
+    status = script_container_->GetStatusOnWorkerThread(script_url);
+  }
+
+  // If the script has not been received at this point, wait for arrival by
+  // blocking the worker thread.
   if (status == ThreadSafeScriptContainer::ScriptStatus::kPending) {
     // Wait for arrival of the script.
     const bool success = script_container_->WaitOnWorkerThread(script_url);
@@ -272,17 +289,9 @@ WebServiceWorkerInstalledScriptsManagerImpl::GetRawScriptData(
 
   if (status == ThreadSafeScriptContainer::ScriptStatus::kFailed)
     return RawScriptData::CreateInvalidInstance();
-  DCHECK_EQ(ThreadSafeScriptContainer::ScriptStatus::kSuccess, status);
-
-  std::unique_ptr<RawScriptData> data =
-      script_container_->TakeOnWorkerThread(script_url);
-  // |data| is possible to be null when the script data has already been taken.
-  if (!data) {
-    // TODO(shimazu): Ask the browser process when the script has already been
-    // served.
-    return nullptr;
-  }
-  return data;
+  DCHECK_EQ(ThreadSafeScriptContainer::ScriptStatus::kReceived, status);
+
+  return script_container_->TakeOnWorkerThread(script_url);
 }
 
 }  // namespace content

content/renderer/service_worker/web_service_worker_installed_scripts_manager_impl.h

@@ -32,10 +32,14 @@ class CONTENT_EXPORT WebServiceWorkerInstalledScriptsManagerImpl final
  private:
   WebServiceWorkerInstalledScriptsManagerImpl(
       std::vector<GURL>&& installed_urls,
-      scoped_refptr<ThreadSafeScriptContainer> script_container);
+      scoped_refptr<ThreadSafeScriptContainer> script_container,
+      mojom::ServiceWorkerInstalledScriptsManagerHostPtr manager_host);
 
   const std::set<GURL> installed_urls_;
   scoped_refptr<ThreadSafeScriptContainer> script_container_;
+
+  scoped_refptr<mojom::ThreadSafeServiceWorkerInstalledScriptsManagerHostPtr>
+      manager_host_;
 };
 
 }  // namespace content

content/renderer/service_worker/web_service_worker_installed_scripts_manager_impl_unittest.cc

@@ -9,32 +9,35 @@
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
-class BrowserSideSender {
+class BrowserSideSender : mojom::ServiceWorkerInstalledScriptsManagerHost {
  public:
-  explicit BrowserSideSender(std::vector<GURL> installed_urls)
-      : installed_urls_(std::move(installed_urls)) {}
+  BrowserSideSender() : binding_(this) {}
+  ~BrowserSideSender() override = default;
 
-  mojom::ServiceWorkerInstalledScriptsInfoPtr CreateAndBind() {
+  mojom::ServiceWorkerInstalledScriptsInfoPtr CreateAndBind(
+      const std::vector<GURL>& installed_urls) {
     EXPECT_FALSE(manager_.is_bound());
     EXPECT_FALSE(body_handle_.is_valid());
     EXPECT_FALSE(meta_data_handle_.is_valid());
     auto scripts_info = mojom::ServiceWorkerInstalledScriptsInfo::New();
-    scripts_info->installed_urls = installed_urls_;
+    scripts_info->installed_urls = installed_urls;
     scripts_info->manager_request = mojo::MakeRequest(&manager_);
+    binding_.Bind(mojo::MakeRequest(&scripts_info->manager_host_ptr));
     return scripts_info;
   }
 
-  const GURL& TransferInstalledScript(int64_t body_size,
-                                      int64_t meta_data_size) {
+  void TransferInstalledScript(const GURL& script_url,
+                               int64_t body_size,
+                               int64_t meta_data_size) {
     EXPECT_FALSE(body_handle_.is_valid());
     EXPECT_FALSE(meta_data_handle_.is_valid());
     auto script_info = mojom::ServiceWorkerScriptInfo::New();
-    const GURL& transferring_url = installed_urls()[next_transfer_index_];
-    script_info->script_url = transferring_url;
+    script_info->script_url = script_url;
     EXPECT_EQ(MOJO_RESULT_OK,
               mojo::CreateDataPipe(nullptr, &body_handle_, &script_info->body));
     EXPECT_EQ(MOJO_RESULT_OK, mojo::CreateDataPipe(nullptr, &meta_data_handle_,
@@ -42,8 +45,6 @@ class BrowserSideSender {
     script_info->body_size = body_size;
     script_info->meta_data_size = meta_data_size;
     manager_->TransferInstalledScript(std::move(script_info));
-    next_transfer_index_++;
-    return transferring_url;
   }
 
   void PushBody(const std::string& data) {
@@ -60,9 +61,20 @@ class BrowserSideSender {
 
   void ResetManager() { manager_.reset(); }
 
-  const std::vector<GURL>& installed_urls() const { return installed_urls_; }
+  void WaitForRequestInstalledScript(const GURL& script_url) {
+    waiting_requested_url_ = script_url;
+    base::RunLoop loop;
+    requested_script_closure_ = loop.QuitClosure();
+    loop.Run();
+  }
 
  private:
+  void RequestInstalledScript(const GURL& script_url) override {
+    EXPECT_EQ(waiting_requested_url_, script_url);
+    ASSERT_TRUE(requested_script_closure_);
+    std::move(requested_script_closure_).Run();
+  }
+
   void PushDataPipe(const std::string& data,
                     const mojo::DataPipeProducerHandle& handle) {
     // Send |data| with null terminator.
@@ -74,10 +86,11 @@ class BrowserSideSender {
     ASSERT_EQ(data.size() + 1, written_bytes);
   }
 
-  const std::vector<GURL> installed_urls_;
-  size_t next_transfer_index_ = 0;
+  base::OnceClosure requested_script_closure_;
+  GURL waiting_requested_url_;
 
   mojom::ServiceWorkerInstalledScriptsManagerPtr manager_;
+  mojo::Binding<mojom::ServiceWorkerInstalledScriptsManagerHost> binding_;
 
   mojo::ScopedDataPipeProducerHandle body_handle_;
   mojo::ScopedDataPipeProducerHandle meta_data_handle_;
@@ -175,8 +188,8 @@ TEST_F(WebServiceWorkerInstalledScriptsManagerImplTest, GetRawScriptData) {
   const GURL kScriptUrl = GURL("https://example.com/installed1.js");
   const GURL kUnknownScriptUrl = GURL("https://example.com/not_installed.js");
 
-  BrowserSideSender sender({kScriptUrl});
-  CreateInstalledScriptsManager(sender.CreateAndBind());
+  BrowserSideSender sender;
+  CreateInstalledScriptsManager(sender.CreateAndBind({kScriptUrl}));
 
   {
     bool result = false;
@@ -202,9 +215,8 @@ TEST_F(WebServiceWorkerInstalledScriptsManagerImplTest, GetRawScriptData) {
         GetRawScriptDataOnWorkerThread(kScriptUrl, &script_data);
 
     // Start transferring the script. +1 for null terminator.
-    EXPECT_EQ(kScriptUrl,
-              sender.TransferInstalledScript(kExpectedBody.size() + 1,
-                                             kExpectedMetaData.size() + 1));
+    sender.TransferInstalledScript(kScriptUrl, kExpectedBody.size() + 1,
+                                   kExpectedMetaData.size() + 1);
     sender.PushBody(kExpectedBody);
     sender.PushMetaData(kExpectedMetaData);
     // GetRawScriptData should be blocked until body and meta data transfer are
@@ -231,10 +243,41 @@ TEST_F(WebServiceWorkerInstalledScriptsManagerImplTest, GetRawScriptData) {
 
   {
     std::unique_ptr<RawScriptData> script_data;
-    GetRawScriptDataOnWorkerThread(kScriptUrl, &script_data)->Wait();
-    // This should not be blocked because the script has already been received.
-    // nullptr will be returned after the data has already been taken.
-    EXPECT_EQ(nullptr, script_data.get());
+    const std::string kExpectedBody = "This is another script body.";
+    const std::string kExpectedMetaData = "This is another meta data.";
+
+    // Request the same script again.
+    base::WaitableEvent* get_raw_script_data_waiter =
+        GetRawScriptDataOnWorkerThread(kScriptUrl, &script_data);
+
+    // It should call a Mojo IPC "RequestInstalledScript()" to the browser.
+    sender.WaitForRequestInstalledScript(kScriptUrl);
+
+    // Start transferring the script. +1 for null terminator.
+    sender.TransferInstalledScript(kScriptUrl, kExpectedBody.size() + 1,
+                                   kExpectedMetaData.size() + 1);
+    sender.PushBody(kExpectedBody);
+    sender.PushMetaData(kExpectedMetaData);
+    // GetRawScriptData should be blocked until body and meta data transfer are
+    // finished.
+    EXPECT_FALSE(get_raw_script_data_waiter->IsSignaled());
+    sender.FinishTransferBody();
+    sender.FinishTransferMetaData();
+
+    // Wait for the script's arrival.
+    get_raw_script_data_waiter->Wait();
+    ASSERT_TRUE(script_data);
+    EXPECT_TRUE(script_data->IsValid());
+    ASSERT_EQ(1u, script_data->ScriptTextChunks().size());
+    ASSERT_EQ(kExpectedBody.size() + 1,
+              script_data->ScriptTextChunks()[0].size());
+    EXPECT_STREQ(kExpectedBody.data(),
+                 script_data->ScriptTextChunks()[0].Data());
+    ASSERT_EQ(1u, script_data->MetaDataChunks().size());
+    ASSERT_EQ(kExpectedMetaData.size() + 1,
+              script_data->MetaDataChunks()[0].size());
+    EXPECT_STREQ(kExpectedMetaData.data(),
+                 script_data->MetaDataChunks()[0].Data());
   }
 }
 
@@ -243,8 +286,8 @@ TEST_F(WebServiceWorkerInstalledScriptsManagerImplTest,
   const GURL kScriptUrl = GURL("https://example.com/installed1.js");
   const GURL kUnknownScriptUrl = GURL("https://example.com/not_installed.js");
 
-  BrowserSideSender sender({kScriptUrl});
-  CreateInstalledScriptsManager(sender.CreateAndBind());
+  BrowserSideSender sender;
+  CreateInstalledScriptsManager(sender.CreateAndBind({kScriptUrl}));
 
   {
     std::unique_ptr<RawScriptData> script_data;
@@ -257,9 +300,8 @@ TEST_F(WebServiceWorkerInstalledScriptsManagerImplTest,
     // Body is expected to be 100 bytes larger than kExpectedBody, but sender
     // only sends kExpectedBody and a null byte (kExpectedBody.size() + 1 bytes
     // in total).
-    EXPECT_EQ(kScriptUrl,
-              sender.TransferInstalledScript(kExpectedBody.size() + 100,
-                                             kExpectedMetaData.size() + 1));
+    sender.TransferInstalledScript(kScriptUrl, kExpectedBody.size() + 100,
+                                   kExpectedMetaData.size() + 1);
     sender.PushBody(kExpectedBody);
     sender.PushMetaData(kExpectedMetaData);
     // GetRawScriptData should be blocked until body and meta data transfer are
@@ -291,8 +333,8 @@ TEST_F(WebServiceWorkerInstalledScriptsManagerImplTest,
   const GURL kScriptUrl = GURL("https://example.com/installed1.js");
   const GURL kUnknownScriptUrl = GURL("https://example.com/not_installed.js");
 
-  BrowserSideSender sender({kScriptUrl});
-  CreateInstalledScriptsManager(sender.CreateAndBind());
+  BrowserSideSender sender;
+  CreateInstalledScriptsManager(sender.CreateAndBind({kScriptUrl}));
 
   {
     std::unique_ptr<RawScriptData> script_data;
@@ -305,9 +347,8 @@ TEST_F(WebServiceWorkerInstalledScriptsManagerImplTest,
     // Meta data is expected to be 100 bytes larger than kExpectedMetaData, but
     // sender only sends kExpectedMetaData and a null byte
     // (kExpectedMetaData.size() + 1 bytes in total).
-    EXPECT_EQ(kScriptUrl,
-              sender.TransferInstalledScript(kExpectedBody.size() + 1,
-                                             kExpectedMetaData.size() + 100));
+    sender.TransferInstalledScript(kScriptUrl, kExpectedBody.size() + 1,
+                                   kExpectedMetaData.size() + 100);
     sender.PushBody(kExpectedBody);
     sender.PushMetaData(kExpectedMetaData);
     // GetRawScriptData should be blocked until body and meta data transfer are
@@ -339,8 +380,8 @@ TEST_F(WebServiceWorkerInstalledScriptsManagerImplTest,
   const GURL kScriptUrl = GURL("https://example.com/installed1.js");
   const GURL kUnknownScriptUrl = GURL("https://example.com/not_installed.js");
 
-  BrowserSideSender sender({kScriptUrl});
-  CreateInstalledScriptsManager(sender.CreateAndBind());
+  BrowserSideSender sender;
+  CreateInstalledScriptsManager(sender.CreateAndBind({kScriptUrl}));
 
   {
     std::unique_ptr<RawScriptData> script_data;

third_party/WebKit/Source/core/workers/InstalledScriptsManager.h

@@ -53,17 +53,14 @@ class InstalledScriptsManager {
   // installed.
   virtual bool IsScriptInstalled(const KURL& script_url) const = 0;
 
-  enum class ScriptStatus { kSuccess, kTaken, kFailed };
+  enum class ScriptStatus { kSuccess, kFailed };
   // Used on the worker thread. GetScriptData() can provide a script for the
   // |script_url| only once. When GetScriptData returns
-  // - ScriptStatus::kSuccess: the script has been received correctly. Sets the
-  //                           script to |out_script_data|.
-  // - ScriptStatus::kTaken: the script has been served from this manager
-  //                         (i.e. the same script is read more than once).
-  //                         |out_script_data| is left as is.
+  // - ScriptStatus::kSuccess: the script has been received correctly. Sets
+  //                           |out_script_data| to the script.
   // - ScriptStatus::kFailed: an error happened while receiving the script from
-  //                          the browser process.
-  //                         |out_script_data| is left as is.
+  //                          the browser process. |out_script_data| is set to
+  //                          empty ScriptData.
   // This can block if the script has not been received from the browser process
   // yet.
   virtual ScriptStatus GetScriptData(const KURL& script_url,

third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp

@@ -254,8 +254,6 @@ WorkerGlobalScope::LoadingScriptFromInstalledScriptsManager(
       GetThread()->GetInstalledScriptsManager()->GetScriptData(script_url,
                                                                &script_data);
   switch (status) {
-    case InstalledScriptsManager::ScriptStatus::kTaken:
-      return LoadResult::kNotHandled;
     case InstalledScriptsManager::ScriptStatus::kFailed:
       return LoadResult::kFailed;
     case InstalledScriptsManager::ScriptStatus::kSuccess:

third_party/WebKit/Source/core/workers/WorkerGlobalScope.h

@@ -177,8 +177,7 @@ class CORE_EXPORT WorkerGlobalScope
       const Vector<CSPHeaderAndType>& headers);
 
   // |kNotHandled| is used when the script was not in
-  // InstalledScriptsManager, which means either it was not an installed script
-  // or it was already taken.
+  // InstalledScriptsManager, which means it was not an installed script.
   enum class LoadResult { kSuccess, kFailed, kNotHandled };
 
   // Tries to load the script synchronously from the

third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp

@@ -116,18 +116,10 @@ void ServiceWorkerGlobalScope::EvaluateClassicScript(
     InstalledScriptsManager::ScriptData script_data;
     InstalledScriptsManager::ScriptStatus status =
         installed_scripts_manager->GetScriptData(script_url, &script_data);
-    switch (status) {
-      case InstalledScriptsManager::ScriptStatus::kTaken:
-        // kTaken should not be returned since requesting the main script should
-        // be the first and no script has been taken until here.
-        NOTREACHED();
-        return;
-      case InstalledScriptsManager::ScriptStatus::kFailed:
-        // This eventually terminates the worker thread.
-        ReportingProxy().DidEvaluateWorkerScript(false);
-        return;
-      case InstalledScriptsManager::ScriptStatus::kSuccess:
-        break;
+    if (status == InstalledScriptsManager::ScriptStatus::kFailed) {
+      // This eventually terminates the worker thread.
+      ReportingProxy().DidEvaluateWorkerScript(false);
+      return;
     }
 
     DCHECK(source_code.IsEmpty());

third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerInstalledScriptsManager.cpp

@@ -29,10 +29,11 @@ ServiceWorkerInstalledScriptsManager::GetScriptData(
   // This blocks until the script is received from the browser.
   std::unique_ptr<WebServiceWorkerInstalledScriptsManager::RawScriptData>
       raw_script_data = manager_->GetRawScriptData(script_url);
-  if (!raw_script_data)
-    return ScriptStatus::kTaken;
-  if (!raw_script_data->IsValid())
+  DCHECK(raw_script_data);
+  if (!raw_script_data->IsValid()) {
+    *out_script_data = InstalledScriptsManager::ScriptData();
     return ScriptStatus::kFailed;
+  }
 
   // This is from WorkerScriptLoader::DidReceiveData.
   std::unique_ptr<TextResourceDecoder> decoder;

third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerInstalledScriptsManager.h

@@ -85,9 +85,8 @@ class WebServiceWorkerInstalledScriptsManager {
 
   // Called on the main or worker thread.
   virtual bool IsScriptInstalled(const blink::WebURL& script_url) const = 0;
-  // Returns the script data for |script_url|. If the data has already been
-  // taken, returns nullptr. When an error happened during receiving the script,
-  // returns an invalid instance.
+  // Returns the script data for |script_url|. When an error happened during
+  // receiving the script, returns an invalid instance.
   // Called on the worker thread.
   virtual std::unique_ptr<RawScriptData> GetRawScriptData(
       const WebURL& script_url) = 0;