Add ref count to service workers for extension API.

chrome/browser/extensions/service_worker_apitest.cc

Index: chrome/browser/extensions/service_worker_apitest.cc
diff --git a/chrome/browser/extensions/service_worker_apitest.cc b/chrome/browser/extensions/service_worker_apitest.cc
index 227584f99c1a1880ce3d64f2582357edeecf58e3..a412c44814e84175b64249dfb9518a6302b00638 100644
--- a/chrome/browser/extensions/service_worker_apitest.cc
+++ b/chrome/browser/extensions/service_worker_apitest.cc
@@ -24,6 +24,8 @@
 #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"
@@ -162,7 +164,29 @@ class ServiceWorkerTest : public ExtensionApiTest {
     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.
@@ -170,6 +194,8 @@ class ServiceWorkerTest : public ExtensionApiTest {
   // removed.
   ScopedCurrentChannel current_channel_;
 
+  size_t last_worker_ref_count_ = 0u;
+
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerTest);
 };
 
@@ -630,6 +656,63 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerTest, TabsCreate) {
   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