Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502
Committed: https://crrev.com/36986071183bdb219186aa9f63f9ee4b162d91b9
Committed: https://crrev.com/519e054087b8d7ab72126075040bb7c819c3d007
Cr-Original-Commit-Position: refs/heads/master@{#407487}
Cr-Commit-Position: refs/heads/master@{#407761}

[horo, 2016-07-25 07:34:18]

The CQ bit was checked by horo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 07:34:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[horo, 2016-07-25 07:50:18]

The CQ bit was checked by horo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 07:50:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[horo, 2016-07-25 08:07:39]

The CQ bit was checked by horo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 08:07:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[horo, 2016-07-25 08:07:59]

Patchset #2 (id:20001) has been deleted

[horo, 2016-07-25 08:08:17]

horo@chromium.org changed reviewers:
+ nhiroki@chromium.org

[horo, 2016-07-25 08:08:18]

nhiroki@
Could you please review?

[commit-bot: I haz the power, 2016-07-25 08:27:42]

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

[commit-bot: I haz the power, 2016-07-25 08:27:43]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[horo, 2016-07-25 09:09:52]

The CQ bit was checked by horo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 09:10:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[nhiroki, 2016-07-25 09:15:40]

https://codereview.chromium.org/2181553003/diff/40001/content/browser/backgro...
File content/browser/background_sync/background_sync_manager_unittest.cc
(right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/backgro...
content/browser/background_sync/background_sync_manager_unittest.cc:71:
*out_registration = registration;
std::move(registration)

https://codereview.chromium.org/2181553003/diff/40001/content/browser/backgro...
File content/browser/background_sync/background_sync_service_impl_unittest.cc
(right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/backgro...
content/browser/background_sync/background_sync_service_impl_unittest.cc:56:
*out_registration = registration;
std::move

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_context_wrapper.cc:590: this,
callback, registration));
Can you make this receive scoped_refptr<Registration> and move |registration|?
This is called only here, so a change should not widely affect.

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_job_unittest.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_job_unittest.cc:64: *registration
= result;
std::move

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:158:
internal_.registration = registration;
std::move

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_storage.cc:216:
installing_registration ? SERVICE_WORKER_OK
This ternary operation may be evaluated after the registration is moved, and its
result is undetermined.

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_storage_unittest.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_storage_unittest.cc:72: *found =
registration;
std::move

[horo, 2016-07-25 09:25:57]

The CQ bit was checked by horo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 09:26:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[horo, 2016-07-25 09:32:02]

https://codereview.chromium.org/2181553003/diff/40001/content/browser/backgro...
File content/browser/background_sync/background_sync_manager_unittest.cc
(right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/backgro...
content/browser/background_sync/background_sync_manager_unittest.cc:71:
*out_registration = registration;
On 2016/07/25 09:15:40, nhiroki (slow) wrote:
> std::move(registration)

Done.

https://codereview.chromium.org/2181553003/diff/40001/content/browser/backgro...
File content/browser/background_sync/background_sync_service_impl_unittest.cc
(right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/backgro...
content/browser/background_sync/background_sync_service_impl_unittest.cc:56:
*out_registration = registration;
On 2016/07/25 09:15:40, nhiroki (slow) wrote:
> std::move

Done.

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_context_wrapper.cc:590: this,
callback, registration));
On 2016/07/25 09:15:40, nhiroki (slow) wrote:
> Can you make this receive scoped_refptr<Registration> and move |registration|?
> This is called only here, so a change should not widely affect.

Done.

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_job_unittest.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_job_unittest.cc:64: *registration
= result;
On 2016/07/25 09:15:40, nhiroki (slow) wrote:
> std::move

Done.

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_register_job.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_register_job.cc:158:
internal_.registration = registration;
On 2016/07/25 09:15:40, nhiroki (slow) wrote:
> std::move

Done.

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_storage.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_storage.cc:216:
installing_registration ? SERVICE_WORKER_OK
On 2016/07/25 09:15:40, nhiroki (slow) wrote:
> This ternary operation may be evaluated after the registration is moved, and
its
> result is undetermined.

Done.

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_storage_unittest.cc (right):

https://codereview.chromium.org/2181553003/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_storage_unittest.cc:72: *found =
registration;
On 2016/07/25 09:15:40, nhiroki (slow) wrote:
> std::move

Done.

[nhiroki, 2016-07-25 09:43:44]

lgtm

[Peter Beverloo, 2016-07-25 10:51:01]

peter@chromium.org changed reviewers:
+ peter@chromium.org

[Peter Beverloo, 2016-07-25 10:51:02]

push and notifications lgtm

[commit-bot: I haz the power, 2016-07-25 10:55:32]

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

[commit-bot: I haz the power, 2016-07-25 10:55:33]

Dry run: This issue passed the CQ dry run.

[horo, 2016-07-25 11:12:44]

horo@chromium.org changed reviewers:
+ dgozman@chromium.org, jkarlin@chromium.org, mmenke@chromium.org

[horo, 2016-07-25 11:12:45]

jkarlin@:
Please review content/browser/background_sync/*

dgozman@:
Please review content/browser/devtools/*

mmenke@:
Please review content/browser/loader/*

[jkarlin, 2016-07-25 11:36:11]

content/browser/background_sync* lgtm

[dgozman, 2016-07-25 14:48:49]

devtools lgtm

[mmenke, 2016-07-25 15:41:43]

On 2016/07/25 14:48:49, dgozman wrote:
> devtools lgtm

content/browser/loader/ LGTM.

[horo, 2016-07-25 15:55:56]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2016-07-25 15:56:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-25 16:00:42]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-07-25 16:01:38]

Description was changed from

==========
Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502
==========

to

==========
Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502

Committed: https://crrev.com/36986071183bdb219186aa9f63f9ee4b162d91b9
Cr-Commit-Position: refs/heads/master@{#407487}
==========

[commit-bot: I haz the power, 2016-07-25 16:01:40]

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

[dominickn, 2016-07-26 05:35:47]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/2183783002/ by dominickn@chromium.org.

The reason for reverting is: Speculative revert to see if this caused
crbug.com/631190. App banners use methods touched by this CL to check if a
service worker is registered; this is the only relevant-looking CL in the
blamelist for the app banner data fetcher tests starting to fail on the Win7 SP1
bot.

This CL will be relanded if re-enabling the disabled app banner data fetcher
tests still causes failures on
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2....

[horo, 2016-07-26 08:33:57]

Description was changed from

==========
Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502

Committed: https://crrev.com/36986071183bdb219186aa9f63f9ee4b162d91b9
Cr-Commit-Position: refs/heads/master@{#407487}
==========

to

==========
Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502

Committed: https://crrev.com/36986071183bdb219186aa9f63f9ee4b162d91b9
Cr-Commit-Position: refs/heads/master@{#407487}
==========

[horo, 2016-07-26 08:35:31]

On 2016/07/26 05:35:47, dominickn wrote:
> A revert of this CL (patchset #4 id:80001) has been created in
> https://codereview.chromium.org/2183783002/ by mailto:dominickn@chromium.org.
> 
> The reason for reverting is: Speculative revert to see if this caused
> crbug.com/631190. App banners use methods touched by this CL to check if a
> service worker is registered; this is the only relevant-looking CL in the
> blamelist for the app banner data fetcher tests starting to fail on the Win7
SP1
> bot.
> 
> This CL will be relanded if re-enabling the disabled app banner data fetcher
> tests still causes failures on
>
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%2....

Sorry, my change caused crbug.com/631190.

In ServiceWorkerContextCore::DidFindRegistrationForCheckHasServiceWorker():
    registration->RegisterRegistrationFinishedCallback(
        base::Bind(&ServiceWorkerContextCore::
                       OnRegistrationFinishedForCheckHasServiceWorker,
                   AsWeakPtr(), callback, std::move(registration)));
We can't call std::move() here.

I updated Patch Set 5.
I will commit it.

[horo, 2016-07-26 08:35:57]

The CQ bit was checked by horo@chromium.org

[horo, 2016-07-26 08:35:58]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org,
jkarlin@chromium.org, dgozman@chromium.org, nhiroki@chromium.org,
mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2181553003/#ps100001
(title: "fix
ServiceWorkerContextCore::DidFindRegistrationForCheckHasServiceWorker()")

[commit-bot: I haz the power, 2016-07-26 08:36:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 10:43:19]

Description was changed from

==========
Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502

Committed: https://crrev.com/36986071183bdb219186aa9f63f9ee4b162d91b9
Cr-Commit-Position: refs/heads/master@{#407487}
==========

to

==========
Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502

Committed: https://crrev.com/36986071183bdb219186aa9f63f9ee4b162d91b9
Cr-Commit-Position: refs/heads/master@{#407487}
==========

[commit-bot: I haz the power, 2016-07-26 10:43:22]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-07-26 10:45:15]

Description was changed from

==========
Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502

Committed: https://crrev.com/36986071183bdb219186aa9f63f9ee4b162d91b9
Cr-Commit-Position: refs/heads/master@{#407487}
==========

to

==========
Stop passing const-reference of SWRegistration in all GetRegistrationsCallbacks.

BUG=616502

Committed: https://crrev.com/36986071183bdb219186aa9f63f9ee4b162d91b9
Committed: https://crrev.com/519e054087b8d7ab72126075040bb7c819c3d007
Cr-Original-Commit-Position: refs/heads/master@{#407487}
Cr-Commit-Position: refs/heads/master@{#407761}
==========

[commit-bot: I haz the power, 2016-07-26 10:45:17]

Patchset 5 (id:??) landed as
https://crrev.com/519e054087b8d7ab72126075040bb7c819c3d007
Cr-Commit-Position: refs/heads/master@{#407761}