Implement memory-persistent registration

content/browser/service_worker/service_worker_storage_unittest.cc

Index: content/browser/service_worker/service_worker_storage_unittest.cc
diff --git a/content/browser/service_worker/service_worker_storage_unittest.cc b/content/browser/service_worker/service_worker_storage_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0d4a22cd8dfe9f804da021a7b903445f464257e4
--- /dev/null
+++ b/content/browser/service_worker/service_worker_storage_unittest.cc
@@ -0,0 +1,383 @@
+// Copyright (c) 2012 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 "content/browser/service_worker/service_worker_storage.h"
+
+#include "base/files/scoped_temp_dir.h"
+#include "base/logging.h"
+#include "base/message_loop/message_loop.h"
+#include "content/browser/browser_thread_impl.h"
+#include "content/browser/service_worker/service_worker_registration.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+class ServiceWorkerStorageTest : public testing::Test {
+ public:
+  ServiceWorkerStorageTest() : io_thread_(BrowserThread::IO, &message_loop_) {}
+
+  virtual void SetUp() OVERRIDE {
+    storage_ = new ServiceWorkerStorage(base::FilePath(), NULL);
+  }
+
+  virtual void TearDown() OVERRIDE {}
+
+ protected:
+  ServiceWorkerStorage* storage_;

[kinuko, 2013/11/27 03:45:23]

scoped_ptr ?  Looks like it's leaking

[alecflett, 2013/12/02 16:11:35]

Done.

+
+  base::MessageLoopForIO message_loop_;
+  BrowserThreadImpl io_thread_;
+};
+
+TEST_F(ServiceWorkerStorageTest, RegisterMatch) {

[kinuko, 2013/11/27 03:45:23]

RegisterMatch -> PatternMatches to make it clearer what method it's testing?

[alecflett, 2013/12/02 16:11:35]

Done.

+  ASSERT_TRUE(ServiceWorkerStorage::PatternMatches(
+      GURL("http://www.example.com/*"), GURL("http://www.example.com/")));
+  ASSERT_TRUE(ServiceWorkerStorage::PatternMatches(
+      GURL("http://www.example.com/*"),
+      GURL("http://www.example.com/page.html")));
+
+  ASSERT_FALSE(ServiceWorkerStorage::PatternMatches(
+      GURL("http://www.example.com/*"), GURL("https://www.example.com/")));
+  ASSERT_FALSE(ServiceWorkerStorage::PatternMatches(
+      GURL("http://www.example.com/*"),
+      GURL("https://www.example.com/page.html")));
+
+  ASSERT_FALSE(ServiceWorkerStorage::PatternMatches(
+      GURL("http://www.example.com/*"), GURL("http://www.foo.com/")));
+  ASSERT_FALSE(ServiceWorkerStorage::PatternMatches(
+      GURL("http://www.example.com/*"), GURL("https://www.foo.com/page.html")));
+}
+
+namespace {

[kinuko, 2013/11/27 03:45:23]

Can we move this anon namespace to the top? (e.g. right after 'namespace content
{')

I'd like to have lower routines/details in another namespace together and
separated from the main test code

[alecflett, 2013/12/02 16:11:35]

Done.

+void SaveRegistrationCallback(
+    ServiceWorkerStorage::RegistrationStatus expected_status,
+    bool* called,
+    scoped_refptr<ServiceWorkerRegistration>* registration,
+    ServiceWorkerStorage::RegistrationStatus status,
+    const scoped_refptr<ServiceWorkerRegistration>& result) {
+  EXPECT_EQ(expected_status, status);
+  *called = true;
+  *registration = result;
+}
+
+// Creates a callback which both keeps track of if it's been called,
+// as well as the resulting registration. Whent the callback is fired,
+// it ensures that the resulting status matches the expectation.
+// 'called' is useful for making sure a sychronous callback is or
+// isn't called.
+ServiceWorkerStorage::RegistrationCallback SaveRegistration(
+    ServiceWorkerStorage::RegistrationStatus expected_status,
+    bool* called,
+    scoped_refptr<ServiceWorkerRegistration>* registration) {
+  *called = false;
+  return base::Bind(
+      &SaveRegistrationCallback, expected_status, called, registration);
+}
+
+void SaveUnregistrationCallback(
+    ServiceWorkerStorage::RegistrationStatus expected_status,
+    bool* called,
+    ServiceWorkerStorage::RegistrationStatus status) {
+  EXPECT_EQ(expected_status, status);
+  *called = true;
+}
+
+ServiceWorkerStorage::UnregistrationCallback SaveUnregistration(
+    ServiceWorkerStorage::RegistrationStatus expected_status,
+    bool* called) {
+  *called = false;
+  return base::Bind(&SaveUnregistrationCallback, expected_status, called);
+}
+
+}  // namespace
+
+TEST_F(ServiceWorkerStorageTest, SameDocumentSameRegistration) {
+  scoped_refptr<ServiceWorkerRegistration> original_registration;
+  bool called;
+  storage_->Register(GURL("http://www.example.com/*"),
+                     GURL("http://www.example.com/service_worker.js"),
+                     SaveRegistration(ServiceWorkerStorage::REGISTRATION_OK,
+                                      &called,
+                                      &original_registration));
+  EXPECT_FALSE(called);
+  message_loop_.RunUntilIdle();

[kinuko, 2013/11/27 03:45:23]

It's deprecated, I think we prefer base::RunLoop().RunUntilIdle() now

[alecflett, 2013/12/02 16:11:35]

I'm not sure how to hook up base::RunLoop() to the IO thread then. What this
test does is create an artificial runloop that acts like it's the IO thread.

[alecflett, 2013/12/03 22:34:09]

Ah! I figured it out... TestBrowserThreadBundle 

+  EXPECT_TRUE(called);
+
+  scoped_refptr<ServiceWorkerRegistration> registration1;
+  storage_->FindRegistrationForDocument(
+      GURL("http://www.example.com/"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &registration1));
+  scoped_refptr<ServiceWorkerRegistration> registration2;
+  storage_->FindRegistrationForDocument(
+      GURL("http://www.example.com/"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &registration2));
+
+  ServiceWorkerRegistration* null_registration(NULL);
+  ASSERT_EQ(null_registration, registration1);
+  ASSERT_EQ(null_registration, registration2);
+  EXPECT_FALSE(called);

[kinuko, 2013/11/27 03:45:23]

If we assume FindRegistrationForDocument could return either synchronously or
asynchronously I think we should drop these pre-operation expectations here.

[alecflett, 2013/12/02 16:11:35]

Well mostly I want to track that behavior - I don't want that to change
arbitrarily without us aware of that possibility. Like you pointed out, we
*might* just want to stick with asynchronicity, but I want this to be a consious
choice (i.e. one such that the developer is aware they've changed the behavior
because they have to change tests)

+  message_loop_.RunUntilIdle();
+  EXPECT_TRUE(called);
+  ASSERT_NE(null_registration, registration1);
+  ASSERT_NE(null_registration, registration2);
+
+  ASSERT_EQ(registration1, registration2);
+}
+
+TEST_F(ServiceWorkerStorageTest, SameMatchSameRegistration) {
+  bool called;
+  scoped_refptr<ServiceWorkerRegistration> original_registration;
+  storage_->Register(GURL("http://www.example.com/*"),
+                     GURL("http://www.example.com/service_worker.js"),
+                     SaveRegistration(ServiceWorkerStorage::REGISTRATION_OK,
+                                      &called,
+                                      &original_registration));
+  EXPECT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  EXPECT_TRUE(called);
+  ASSERT_NE(static_cast<ServiceWorkerRegistration*>(NULL),
+            original_registration.get());
+
+  scoped_refptr<ServiceWorkerRegistration> registration1;
+  storage_->FindRegistrationForDocument(
+      GURL("http://www.example.com/one"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &registration1));
+
+  EXPECT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  EXPECT_TRUE(called);
+
+  scoped_refptr<ServiceWorkerRegistration> registration2;
+  storage_->FindRegistrationForDocument(
+      GURL("http://www.example.com/two"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &registration2));
+  EXPECT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  EXPECT_TRUE(called);
+
+  ASSERT_EQ(registration1, registration2);
+}
+
+TEST_F(ServiceWorkerStorageTest, DifferentMatchDifferentRegistration) {
+  bool called1;
+  scoped_refptr<ServiceWorkerRegistration> original_registration1;
+  storage_->Register(GURL("http://www.example.com/one/*"),
+                     GURL("http://www.example.com/service_worker.js"),
+                     SaveRegistration(ServiceWorkerStorage::REGISTRATION_OK,
+                                      &called1,
+                                      &original_registration1));
+
+  bool called2;
+  scoped_refptr<ServiceWorkerRegistration> original_registration2;
+  storage_->Register(GURL("http://www.example.com/two/*"),
+                     GURL("http://www.example.com/service_worker.js"),
+                     SaveRegistration(ServiceWorkerStorage::REGISTRATION_OK,
+                                      &called2,
+                                      &original_registration2));
+
+  EXPECT_FALSE(called1);
+  EXPECT_FALSE(called2);
+  message_loop_.RunUntilIdle();
+  EXPECT_TRUE(called2);
+  EXPECT_TRUE(called1);
+
+  scoped_refptr<ServiceWorkerRegistration> registration1;
+  storage_->FindRegistrationForDocument(
+      GURL("http://www.example.com/one/"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called1, &registration1));
+  scoped_refptr<ServiceWorkerRegistration> registration2;
+  storage_->FindRegistrationForDocument(
+      GURL("http://www.example.com/two/"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called2, &registration2));
+
+  EXPECT_FALSE(called1);
+  EXPECT_FALSE(called2);
+  message_loop_.RunUntilIdle();
+  EXPECT_TRUE(called2);
+  EXPECT_TRUE(called1);
+
+  ASSERT_NE(registration1, registration2);
+}
+
+// Make sure basic registration is working.
+TEST_F(ServiceWorkerStorageTest, Register) {
+  bool called = false;
+  scoped_refptr<ServiceWorkerRegistration> registration;
+  storage_->Register(
+      GURL("http://www.example.com/*"),
+      GURL("http://www.example.com/service_worker.js"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &registration));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_NE(scoped_refptr<ServiceWorkerRegistration>(NULL), registration);

[kinuko, 2013/11/27 03:45:23]

Can we test FindRegistrationForPattern returns OK here too?

[alecflett, 2013/12/02 16:11:35]

oops, I think that was there before I shuffled this code around. Done.

+}
+
+// Make sure registrations are cleaned up when they are unregistered.
+TEST_F(ServiceWorkerStorageTest, Unregister) {
+  GURL pattern("http://www.example.com/*");
+
+  bool called;
+  scoped_refptr<ServiceWorkerRegistration> registration;
+  storage_->Register(
+      pattern,
+      GURL("http://www.example.com/service_worker.js"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &registration));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  storage_->Unregister(
+      pattern,
+      SaveUnregistration(ServiceWorkerStorage::REGISTRATION_OK, &called));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_TRUE(registration->HasOneRef());
+
+  storage_->FindRegistrationForPattern(
+      pattern,
+      SaveRegistration(ServiceWorkerStorage::REGISTRATION_NOT_FOUND,
+                       &called,
+                       &registration));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_EQ(scoped_refptr<ServiceWorkerRegistration>(NULL), registration);
+}
+
+// Make sure that when a new registration replaces an existing
+// registration, that the old one is cleaned up.
+TEST_F(ServiceWorkerStorageTest, RegisterNewScript) {
+  GURL pattern("http://www.example.com/*");
+
+  bool called;
+  scoped_refptr<ServiceWorkerRegistration> old_registration;
+  storage_->Register(
+      pattern,
+      GURL("http://www.example.com/service_worker.js"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &old_registration));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  scoped_refptr<ServiceWorkerRegistration> old_registration_by_pattern;
+  storage_->FindRegistrationForPattern(
+      pattern,
+      SaveRegistration(ServiceWorkerStorage::REGISTRATION_OK,
+                       &called,
+                       &old_registration_by_pattern));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_EQ(old_registration, old_registration_by_pattern);
+  old_registration_by_pattern = NULL;
+
+  scoped_refptr<ServiceWorkerRegistration> new_registration;
+  storage_->Register(
+      pattern,
+      GURL("http://www.example.com/service_worker_new.js"),
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &new_registration));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_TRUE(old_registration->HasOneRef());
+
+  ASSERT_NE(old_registration, new_registration);
+
+  scoped_refptr<ServiceWorkerRegistration> new_registration_by_pattern;
+  storage_->FindRegistrationForPattern(
+      pattern,
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &new_registration));

[kinuko, 2013/11/27 03:45:23]

new_registration -> new_registration_by_pattern ?

[alecflett, 2013/12/02 16:11:35]

oops! typo.
Done.

+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_NE(new_registration_by_pattern, old_registration);
+}
+
+// Make sure that when registering a duplicate pattern+script_url
+// combination, that the same registration is used.
+TEST_F(ServiceWorkerStorageTest, RegisterDuplicateScript) {
+  GURL pattern("http://www.example.com/*");
+  GURL script_url("http://www.example.com/service_worker.js");
+
+  bool called;
+  scoped_refptr<ServiceWorkerRegistration> old_registration;
+  storage_->Register(
+      pattern,
+      script_url,
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &old_registration));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  scoped_refptr<ServiceWorkerRegistration> old_registration_by_pattern;
+  storage_->FindRegistrationForPattern(
+      pattern,
+      SaveRegistration(ServiceWorkerStorage::REGISTRATION_OK,
+                       &called,
+                       &old_registration_by_pattern));
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_TRUE(old_registration_by_pattern);
+
+  scoped_refptr<ServiceWorkerRegistration> new_registration;
+  storage_->Register(
+      pattern,
+      script_url,
+      SaveRegistration(
+          ServiceWorkerStorage::REGISTRATION_OK, &called, &new_registration));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_EQ(old_registration, new_registration);
+
+  ASSERT_FALSE(old_registration->HasOneRef());
+
+  scoped_refptr<ServiceWorkerRegistration> new_registration_by_pattern;
+  storage_->FindRegistrationForPattern(
+      pattern,
+      SaveRegistration(ServiceWorkerStorage::REGISTRATION_OK,
+                       &called,
+                       &new_registration_by_pattern));
+
+  ASSERT_FALSE(called);
+  message_loop_.RunUntilIdle();
+  ASSERT_TRUE(called);
+
+  ASSERT_EQ(new_registration, old_registration);

[kinuko, 2013/11/27 03:45:23]

new_registration -> new_registration_by_pattern ?

[alecflett, 2013/12/02 16:11:35]

Done.

+}
+
+}  // namespace content