Add ref count to service workers for extension API.

chrome/browser/extensions/service_worker_apitest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stdint.h>
 
 #include "base/bind_helpers.h"
 #include "base/macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/notifications/desktop_notification_profile_util.h"
 #include "chrome/browser/permissions/permission_manager.h"
 #include "chrome/browser/push_messaging/push_messaging_app_identifier.h"
 #include "chrome/browser/push_messaging/push_messaging_service_factory.h"
 #include "chrome/browser/push_messaging/push_messaging_service_impl.h"
 #include "chrome/browser/services/gcm/fake_gcm_profile_service.h"
 #include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/permission_type.h"
+#include "content/public/browser/service_worker_context.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/origin_util.h"
 #include "content/public/common/page_type.h"
 #include "content/public/test/background_sync_test_util.h"
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/test/background_page_watcher.h"
@@ skipping 118 matching lines @@
     }
     return inner_text;
   }
 
   // Navigates the browser to |url|, then returns the innerText of the new
   // tab's WebContents' main frame.
   std::string NavigateAndExtractInnerText(const GURL& url) {
     return ExtractInnerText(Navigate(url));
   }
 
+  size_t GetWorkerRefCount(const GURL& origin) {
+    content::ServiceWorkerContext* sw_context =
+        content::BrowserContext::GetDefaultStoragePartition(
+            browser()->profile())
+            ->GetServiceWorkerContext();
+    base::RunLoop run_loop;
+    sw_context->GetPendingExternalRequestCountForTest(
+        origin,
+        base::Bind(&ServiceWorkerTest::DidGetPendingExternalRequestCount,
+                   // Unretained is safe because we wait until the bound method
+                   // is called.
+                   base::Unretained(this), run_loop.QuitClosure()));

[Devlin, 2016/10/07 19:06:44]

Optional nit: you might be able to do this in a lambda instead with something
like

size_t ref_count = 0;
base::RunLoop run_loop;
auto set_ref_count = [&ref_count, &run_loop](size_t count) {
  *ref_count = count;
  run_loop->Quit();
}

That might be a bit cleaner?  But I don't feel strongly either way.

[lazyboy, 2016/10/07 21:28:41]

Ya, less members and callback functions outside.
Done.

+    run_loop.Run();
+    return last_worker_ref_count_;
+  }
+
  private:
+  void DidGetPendingExternalRequestCount(const base::Closure& callback,
+                                         size_t external_request_count) {
+    last_worker_ref_count_ = external_request_count;
+    callback.Run();
+  }
+
   // Sets the channel to "stable".
   // Not useful after we've opened extension Service Workers to stable
   // channel.
   // TODO(lazyboy): Remove this when ExtensionServiceWorkersEnabled() is
   // removed.
   ScopedCurrentChannel current_channel_;
 
+  size_t last_worker_ref_count_ = 0u;
+
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerTest);
 };
 
 class ServiceWorkerBackgroundSyncTest : public ServiceWorkerTest {
  public:
   ServiceWorkerBackgroundSyncTest() {}
   ~ServiceWorkerBackgroundSyncTest() override {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     // ServiceWorkerRegistration.sync requires experimental flag.
@@ skipping 440 matching lines @@
   ASSERT_TRUE(content::ExecuteScriptAndExtractString(
       web_contents, "window.runServiceWorker()", &result));
   ASSERT_EQ("chrome.tabs.create callback", result);
   EXPECT_EQ(starting_tab_count + 1, browser()->tab_strip_model()->count());
 
   // Check extension shutdown path.
   UnloadExtension(extension->id());
   EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
 }
 
+// Tests that worker ref count increments while extension API function is
+// active.
+IN_PROC_BROWSER_TEST_F(ServiceWorkerTest, WorkerRefCount) {
+  // Extensions APIs from SW are only enabled on trunk.
+  ScopedCurrentChannel current_channel_override(version_info::Channel::UNKNOWN);
+  const Extension* extension = LoadExtensionWithFlags(
+      test_data_dir_.AppendASCII("service_worker/api_worker_ref_count"),
+      kFlagNone);
+  ASSERT_TRUE(extension);
+  ui_test_utils::NavigateToURL(browser(),
+                               extension->GetResourceURL("page.html"));
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  ExtensionTestMessageListener worker_start_listener("WORKER STARTED", false);
+  worker_start_listener.set_failure_message("FAILURE");
+  ASSERT_TRUE(
+      content::ExecuteScript(web_contents, "window.runServiceWorker()"));
+  ASSERT_TRUE(worker_start_listener.WaitUntilSatisfied());
+
+  // Service worker should have no pending requests because it hasn't peformed
+  // any extension API request yet.
+  EXPECT_EQ(0u, GetWorkerRefCount(extension->url()));
+
+  ExtensionTestMessageListener worker_listener("CHECK_REF_COUNT", true);
+  worker_listener.set_failure_message("FAILURE");
+  ASSERT_TRUE(content::ExecuteScript(web_contents, "window.testSendMessage()"));
+  ASSERT_TRUE(worker_listener.WaitUntilSatisfied());
+
+  // Service worker should have exactly one pending request because
+  // chrome.test.sendMessage() API call is in-flight.
+  EXPECT_EQ(1u, GetWorkerRefCount(extension->url()));
+
+  // Peform another extension API request while one is ongoing.
+  {
+    ExtensionTestMessageListener listener("CHECK_REF_COUNT", true);
+    listener.set_failure_message("FAILURE");
+    ASSERT_TRUE(
+        content::ExecuteScript(web_contents, "window.testSendMessage()"));
+    ASSERT_TRUE(listener.WaitUntilSatisfied());
+    // Finish executing the nested chrome.test.sendMessage() first.
+    listener.Reply("Hello world");

[Devlin, 2016/10/07 19:06:44]

TL;DR: Put the EXPECT_EQ(2u, GetWorkerRefCount(extension->url())) above the
Reply() call.

this strikes me as conceptually racy, even though I don't think it technically
is.  But since replying completes the sendMessage(), the ref count could
potentially drop any time after this.  We don't have to worry since it's handled
on the renderer side and the process hop takes time, but if we moved the
lifetime management to the browser (something I could see happening), this would
fail.

[lazyboy, 2016/10/07 21:28:41]

Good catch.
Done.

+  }
+
+  // Service worker currently has two extension API requests in-flight.
+  EXPECT_EQ(2u, GetWorkerRefCount(extension->url()));
+
+  ExtensionTestMessageListener extension_listener("SUCCESS", false);
+  extension_listener.set_failure_message("FAILURE");
+  // Finish executing chrome.test.sendMessage().
+  worker_listener.Reply("Hello world");
+  ASSERT_TRUE(extension_listener.WaitUntilSatisfied());
+
+  // The ref count should drop to 0.
+  EXPECT_EQ(0u, GetWorkerRefCount(extension->url()));
+}
+
 // This test loads a web page that has an iframe pointing to a
 // chrome-extension:// URL. The URL is listed in the extension's
 // web_accessible_resources. Initially the iframe is served from the extension's
 // resource file. After verifying that, we register a Service Worker that
 // controls the extension. Further requests to the same resource as before
 // should now be served by the Service Worker.
 // This test also verifies that if the requested resource exists in the manifest
 // but is not present in the extension directory, the Service Worker can still
 // serve the resource file.
 IN_PROC_BROWSER_TEST_F(ServiceWorkerTest, WebAccessibleResourcesIframeSrc) {
@@ skipping 140 matching lines @@
   message.sender_id = "1234567890";
   message.raw_data = "testdata";
   message.decrypted = true;
   push_service()->SetMessageCallbackForTesting(run_loop.QuitClosure());
   push_service()->OnMessage(app_identifier.app_id(), message);
   EXPECT_TRUE(push_message_listener.WaitUntilSatisfied());
   run_loop.Run();  // Wait until the message is handled by push service.
 }
 
 }  // namespace extensions