Decouple script_url from ServiceWorkerRegistration

Decouple script URL from ServiceWorkerRegistration

Before this patch, ServiceWorkerRegistration had a constant
script URL.
The spec has a different model: .installing, .waiting, and .active
workers for the same registration can be from different script URLs.

This patch removes script_url from SWRegistration, in preparation
for implementing the spec steps for same-scope, new-script-url
registration.

BUG=398355
Committed: https://crrev.com/9f2188e301af9fdaf1454703b610234413134361
Cr-Commit-Position: refs/heads/master@{#291882}

[falken, 2014-08-22 10:58:05]

michaeln, nhiroki: everything
horo: resources/service_worker

[falken, 2014-08-25 15:03:51]

ping!

[michaeln, 2014-08-25 22:00:58]

https://codereview.chromium.org/501453002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_register_job.cc (left):

https://codereview.chromium.org/501453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.cc:47:
script_url_(registration->script_url()),
Can this data member can remain const and be reliably set at all times?

  script_url_(registration()->GetNewestVersion()->script_url());

Othewise the ServiceWorkerRegisterJob::Equals comparison function does not work.
I think we can assert that GetNewestVersion() is not null since if there is no
newest version, there can't be anything to update.

[michaeln, 2014-08-25 22:12:17]

> Can this data member can remain const and be reliably set at all times?
> 
>   script_url_(registration()->GetNewestVersion()->script_url());
> 
> Othewise the ServiceWorkerRegisterJob::Equals comparison function does not
work.
> I think we can assert that GetNewestVersion() is not null since if there is no
> newest version, there can't be anything to update.

otherwise the patch looks pretty good!

[falken, 2014-08-26 01:02:28]

https://codereview.chromium.org/501453002/diff/1/content/browser/service_work...
File content/browser/service_worker/service_worker_register_job.cc (left):

https://codereview.chromium.org/501453002/diff/1/content/browser/service_work...
content/browser/service_worker/service_worker_register_job.cc:47:
script_url_(registration->script_url()),
On 2014/08/25 22:00:58, michaeln wrote:
> Can this data member can remain const and be reliably set at all times?
> 
>   script_url_(registration()->GetNewestVersion()->script_url());
> 
> Othewise the ServiceWorkerRegisterJob::Equals comparison function does not
work.
> I think we can assert that GetNewestVersion() is not null since if there is no
> newest version, there can't be anything to update.

Great catch. But we must handle this scenario:

1. Queue a Register job for same-scope, new script URL
2. Queue an Update job, it sets script_url_ to current script
3. Register job makes a waiting version with new script
4. Update job will make an installing version with the old script

I think Update can just abort when it starts if its script_url_ is not the
newest version's.

[horo, 2014-08-26 02:08:22]

lgtm

[falken, 2014-08-26 02:15:02]

michaeln: PTAL, addressed your comment and added some unittests
horo: thx

[michaeln, 2014-08-26 03:05:53]

https://chromiumcodereview.appspot.com/501453002/diff/40001/content/browser/s...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://chromiumcodereview.appspot.com/501453002/diff/40001/content/browser/s...
content/browser/service_worker/service_worker_register_job.cc:239:
Complete(SERVICE_WORKER_ERROR_NOT_FOUND);
lgtm, thnx

[falken, 2014-08-26 04:01:29]

The CQ bit was checked by falken@chromium.org

[commit-bot: I haz the power, 2014-08-26 04:02:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/falken@chromium.org/501453002/40001

[commit-bot: I haz the power, 2014-08-26 05:02:02]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[commit-bot: I haz the power, 2014-08-26 06:43:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-26 06:43:38]

Failed to apply patch for
content/browser/service_worker/service_worker_browsertest.cc:
While running git apply --index -p1;
  error: patch failed:
content/browser/service_worker/service_worker_browsertest.cc:494
  error: content/browser/service_worker/service_worker_browsertest.cc: patch
does not apply

Patch:       content/browser/service_worker/service_worker_browsertest.cc
Index: content/browser/service_worker/service_worker_browsertest.cc
diff --git a/content/browser/service_worker/service_worker_browsertest.cc
b/content/browser/service_worker/service_worker_browsertest.cc
index
1e8450cf012bd5d79a20c95a0db861f80715ab29..c55fe26d51e76569c2931120dcba1eec2c880a51
100644
--- a/content/browser/service_worker/service_worker_browsertest.cc
+++ b/content/browser/service_worker/service_worker_browsertest.cc
@@ -494,11 +494,11 @@ class ServiceWorkerVersionBrowserTest : public
ServiceWorkerBrowserTest {
   void SetUpRegistrationOnIOThread(const std::string& worker_url) {
     registration_ = new ServiceWorkerRegistration(
         embedded_test_server()->GetURL("/"),
-        embedded_test_server()->GetURL(worker_url),
         wrapper()->context()->storage()->NewRegistrationId(),
         wrapper()->context()->AsWeakPtr());
     version_ = new ServiceWorkerVersion(
         registration_,
+        embedded_test_server()->GetURL(worker_url),
         wrapper()->context()->storage()->NewVersionId(),
         wrapper()->context()->AsWeakPtr());
     AssociateRendererProcessToWorker(version_->embedded_worker());
@@ -823,29 +823,23 @@ class ServiceWorkerBlackBoxBrowserTest : public
ServiceWorkerBrowserTest {
 
   void FindRegistrationOnIO(const GURL& document_url,
                             ServiceWorkerStatusCode* status,
-                            GURL* script_url,
                             const base::Closure& continuation) {
     wrapper()->context()->storage()->FindRegistrationForDocument(
         document_url,
         base::Bind(&ServiceWorkerBlackBoxBrowserTest::FindRegistrationOnIO2,
                    this,
                    status,
-                   script_url,
                    continuation));
   }
 
   void FindRegistrationOnIO2(
       ServiceWorkerStatusCode* out_status,
-      GURL* script_url,
       const base::Closure& continuation,
       ServiceWorkerStatusCode status,
       const scoped_refptr<ServiceWorkerRegistration>& registration) {
     *out_status = status;
-    if (registration) {
-      *script_url = registration->script_url();
-    } else {
+    if (!registration)
       EXPECT_NE(SERVICE_WORKER_OK, status);
-    }
     continuation.Run();
   }
 };
@@ -934,13 +928,11 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerBlackBoxBrowserTest,
MAYBE_Registration) {
   // Should not be able to find it.
   {
     ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
-    GURL script_url;
     RunOnIOThread(
         base::Bind(&ServiceWorkerBlackBoxBrowserTest::FindRegistrationOnIO,
                    this,
                   
embedded_test_server()->GetURL("/service_worker/empty.html"),
-                   &status,
-                   &script_url));
+                   &status));
     EXPECT_EQ(SERVICE_WORKER_ERROR_NOT_FOUND, status);
   }
 }

[falken, 2014-08-26 07:16:58]

The CQ bit was checked by falken@chromium.org

[commit-bot: I haz the power, 2014-08-26 07:18:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/falken@chromium.org/501453002/60001

[commit-bot: I haz the power, 2014-08-26 08:45:49]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-26 10:24:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-26 10:24:52]

Try jobs failed on following builders:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[falken, 2014-08-26 10:26:48]

The CQ bit was checked by falken@chromium.org

[commit-bot: I haz the power, 2014-08-26 10:27:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/falken@chromium.org/501453002/60001

[commit-bot: I haz the power, 2014-08-26 11:09:27]

Committed patchset #4 (60001) as 6cce6b78f0872d2e2f3805b7bb69770d5bf7e97d

[commit-bot: I haz the power, 2014-09-10 02:42:02]

Patchset 4 (id:??) landed as
https://crrev.com/9f2188e301af9fdaf1454703b610234413134361
Cr-Commit-Position: refs/heads/master@{#291882}