New issue
Advanced search Search tips

Issue 755477 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Convert callbacks / closures used in ServiceWorker code to use OnceCallback / OnceClosure

Project Member Reported by kinuko@chromium.org, Aug 15 2017

Issue description

No need to do this in one shot (also tzik@'s making some bulk changes with his rewriter for simple patterns), but there're some easier ones that can also contribute to cleaner code:

E.g. ServiceWorkerVersion::RunAfterStartWorker can take OnceClosure instead of Closure.

Note that non-once closure/callback can be implicitly converted to that of once version (i.e. caller can keep calling the code with non-once versions at least during the transition), while the opposite is not true.

https://chromium.googlesource.com/chromium/src/+/lkcr/docs/callback.md
 

Comment 1 by falken@chromium.org, Aug 17 2017

Another good task is to remove base::Passed() that are no longer needed after this CL: https://chromium-review.googlesource.com/613081
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3

commit 0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3
Author: Yannic Bonenberger <contact@yannic-bonenberger.com>
Date: Mon Aug 21 02:54:04 2017

Convert callbacks in ServiceWorker code to OnceCallback

Bug: 755477
Change-Id: Icf09688934dcf4a1e21e76ef0161af9d5a199542
Reviewed-on: https://chromium-review.googlesource.com/615562
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495863}
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/background_fetch/background_fetch_event_dispatcher.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/background_sync/background_sync_manager.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/notifications/notification_event_dispatcher_impl.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/payments/payment_app_provider_impl.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/push_messaging/push_messaging_router.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_controllee_request_handler.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_fetch_dispatcher.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_registration.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_version.h
[modify] https://crrev.com/0e2d7ce54d40fdf86f4c164bc77f6cfe04c88bc3/content/browser/service_worker/service_worker_version_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5cb428c6d0e86d4ce42ee7522816c216aef0cea

commit c5cb428c6d0e86d4ce42ee7522816c216aef0cea
Author: Matt Falkenhagen <falken@chromium.org>
Date: Thu Sep 07 08:43:42 2017

service worker: More OnceCallback migration in ServiceWorkerContext.

Also do other cleanups in comments, etc.

Bug: 755477
TBR: lazyboy@ (extensions), markusheintz@  (browsing_data) [trivial changes]
Change-Id: Ib0a7ae68497a9b10970f582522a359b4611f11ff
Reviewed-on: https://chromium-review.googlesource.com/652729
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500247}
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/chrome/browser/browsing_data/browsing_data_service_worker_helper.cc
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/chrome/browser/extensions/service_worker_apitest.cc
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/content/browser/service_worker/service_worker_context_core.cc
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/content/browser/service_worker/service_worker_context_core.h
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/content/browser/service_worker/service_worker_context_wrapper.h
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/content/public/browser/service_worker_context.h
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/content/public/test/mock_service_worker_context.cc
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/content/public/test/mock_service_worker_context.h
[modify] https://crrev.com/c5cb428c6d0e86d4ce42ee7522816c216aef0cea/content/shell/browser/layout_test/blink_test_controller.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a

commit da8af67cd4d82b017a1b46e878a85d9f8d50fa8a
Author: Matt Falkenhagen <falken@chromium.org>
Date: Tue Sep 12 02:39:23 2017

service worker: Finish OnceCallback migration in ServiceWorkerContext

This also removes the gmock for ServiceWorkerContext since OnceCallback
cannot be copied.

Bug: 755477
Change-Id: I59605c4ebc2d9aa691c73b7140d5e1c86e681198
TBR: mpearson
Reviewed-on: https://chromium-review.googlesource.com/654502
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501158}
[modify] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/chrome/browser/autocomplete/chrome_autocomplete_provider_client.cc
[modify] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/chrome/browser/autocomplete/chrome_autocomplete_provider_client_unittest.cc
[modify] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/chrome/browser/chrome_service_worker_browsertest.cc
[modify] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/content/browser/service_worker/service_worker_context_wrapper.h
[modify] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/content/public/browser/service_worker_context.h
[add] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/content/public/test/fake_service_worker_context.cc
[add] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/content/public/test/fake_service_worker_context.h
[delete] https://crrev.com/707023d380f9c090b21bce9d0f4cb502cce2858a/content/public/test/mock_service_worker_context.cc
[delete] https://crrev.com/707023d380f9c090b21bce9d0f4cb502cce2858a/content/public/test/mock_service_worker_context.h
[modify] https://crrev.com/da8af67cd4d82b017a1b46e878a85d9f8d50fa8a/content/test/BUILD.gn

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f39aeb51da1dba4d008f2c3c1f15373a9459138

commit 4f39aeb51da1dba4d008f2c3c1f15373a9459138
Author: Yannic Bonenberger <contact@yannic-bonenberger.com>
Date: Fri Sep 22 04:11:21 2017

Convert more Callbacks to OnceCallbacks in ServiceWorker code

Everything in ServiceWorkerVersion except SimpleEventCallback is
Once{Closure, Callback} now.

Bug: 755477
Change-Id: I16804692002c2611a66b16f2a529830baf1d7685
Reviewed-on: https://chromium-review.googlesource.com/633607
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Kinuko Yasuda (slow) <kinuko@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503655}
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/background_fetch/background_fetch_event_dispatcher.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/background_sync/background_sync_manager.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/background_sync/background_sync_manager.h
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/background_sync/background_sync_manager_unittest.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/notifications/notification_event_dispatcher_impl.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/payments/payment_app_provider_impl.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/push_messaging/push_messaging_router.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/browser_side_controller_service_worker.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/browser_side_controller_service_worker.h
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/embedded_worker_instance.h
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/embedded_worker_instance_unittest.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/foreign_fetch_request_handler_unittest.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_cache_writer.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_cache_writer.h
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_cache_writer_unittest.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_fetch_dispatcher.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_internals_ui.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_internals_ui.h
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_registration.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_url_request_job_unittest.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_version.h
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_version_unittest.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/browser/service_worker/service_worker_write_to_cache_job.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/test/test_background_sync_manager.cc
[modify] https://crrev.com/4f39aeb51da1dba4d008f2c3c1f15373a9459138/content/test/test_background_sync_manager.h

Comment 6 by falken@chromium.org, Sep 29 2017

Note: ServiceWorkerStorage and ServiceWorkerDatabase are good next targets.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/276f0bfed01fe015037c53fe0060d61ea219c982

commit 276f0bfed01fe015037c53fe0060d61ea219c982
Author: Yannic Bonenberger <contact@yannic-bonenberger.com>
Date: Wed Nov 22 11:43:15 2017

Prepare BackgroundSyncManager unittest for removal of ServiceWorkerVersion::LegacyStatusCallback

Bug: 755477
Change-Id: Ib2055bea28420629e59bd95c5fa0cd0bbf34cf7e
Reviewed-on: https://chromium-review.googlesource.com/688616
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518608}
[modify] https://crrev.com/276f0bfed01fe015037c53fe0060d61ea219c982/content/browser/background_sync/background_sync_manager_unittest.cc
[modify] https://crrev.com/276f0bfed01fe015037c53fe0060d61ea219c982/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/276f0bfed01fe015037c53fe0060d61ea219c982/content/test/test_background_sync_manager.cc
[modify] https://crrev.com/276f0bfed01fe015037c53fe0060d61ea219c982/content/test/test_background_sync_manager.h

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb28a0bc4645ac502f8476859fece518180443bb

commit eb28a0bc4645ac502f8476859fece518180443bb
Author: Yannic Bonenberger <contact@yannic-bonenberger.com>
Date: Wed Dec 06 02:31:24 2017

Convert callbacks in ServiceWorkerStorage

Bug: 755477
Change-Id: Ia8554341cf4a27b8b64367c5fb40f3cda689f74d
Reviewed-on: https://chromium-review.googlesource.com/688954
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521962}
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/background_sync/background_sync_manager.cc
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/background_sync/background_sync_manager.h
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/service_worker/service_worker_context_wrapper.h
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/service_worker/service_worker_job_unittest.cc
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/service_worker/service_worker_storage.cc
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/service_worker/service_worker_storage.h
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/browser/service_worker/service_worker_storage_unittest.cc
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/test/test_background_sync_manager.cc
[modify] https://crrev.com/eb28a0bc4645ac502f8476859fece518180443bb/content/test/test_background_sync_manager.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12916246787938e0850a71cb23c17a27e029799a

commit 12916246787938e0850a71cb23c17a27e029799a
Author: Yannic Bonenberger <contact@yannic-bonenberger.com>
Date: Tue Jan 09 09:52:14 2018

Finish OnceCallback migration in ServiceWorkerVersion

Bug: 755477
Bug: 730593
Change-Id: I9b98b5a01d5ebb5fb8128b2a72d6eb2dc44cbf8f
Reviewed-on: https://chromium-review.googlesource.com/688955
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527948}
[modify] https://crrev.com/12916246787938e0850a71cb23c17a27e029799a/content/browser/background_sync/background_sync_manager.cc
[modify] https://crrev.com/12916246787938e0850a71cb23c17a27e029799a/content/browser/background_sync/background_sync_manager.h
[modify] https://crrev.com/12916246787938e0850a71cb23c17a27e029799a/content/browser/devtools/protocol/service_worker_handler.cc
[modify] https://crrev.com/12916246787938e0850a71cb23c17a27e029799a/content/browser/notifications/notification_event_dispatcher_impl.cc
[modify] https://crrev.com/12916246787938e0850a71cb23c17a27e029799a/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/12916246787938e0850a71cb23c17a27e029799a/content/browser/service_worker/service_worker_version.h
[modify] https://crrev.com/12916246787938e0850a71cb23c17a27e029799a/content/test/test_background_sync_manager.cc
[modify] https://crrev.com/12916246787938e0850a71cb23c17a27e029799a/content/test/test_background_sync_manager.h

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d07754fc03da97240e0408a9258db85057650203

commit d07754fc03da97240e0408a9258db85057650203
Author: Yannic Bonenberger <contact@yannic-bonenberger.com>
Date: Mon Sep 10 09:12:20 2018

Remove remaining occurrences of base::Closure from ServiceWorker code

Bug: 755477
Change-Id: I59ca20a9d958a2bcad929b9a83b1ab9053ee8d42
Reviewed-on: https://chromium-review.googlesource.com/1212062
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Yannic Bonenberger <contact@yannic-bonenberger.com>
Cr-Commit-Position: refs/heads/master@{#589866}
[modify] https://crrev.com/d07754fc03da97240e0408a9258db85057650203/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/d07754fc03da97240e0408a9258db85057650203/content/browser/service_worker/service_worker_job_unittest.cc
[modify] https://crrev.com/d07754fc03da97240e0408a9258db85057650203/content/browser/service_worker/service_worker_read_from_cache_job_unittest.cc
[modify] https://crrev.com/d07754fc03da97240e0408a9258db85057650203/content/browser/service_worker/service_worker_registration.cc
[modify] https://crrev.com/d07754fc03da97240e0408a9258db85057650203/content/browser/service_worker/service_worker_registration.h
[modify] https://crrev.com/d07754fc03da97240e0408a9258db85057650203/content/browser/service_worker/service_worker_storage_unittest.cc
[modify] https://crrev.com/d07754fc03da97240e0408a9258db85057650203/content/browser/service_worker/service_worker_test_utils.h

Sign in to add a comment