Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(114)

Issue 327243003: Introduce ProfileInvalidationProvider wrapper for InvalidationService (Closed)

Created:
6 years, 6 months ago by bartfab (slow)
Modified:
6 years, 6 months ago
CC:
chromium-reviews, tim+watch_chromium.org, chromium-apps-reviews_chromium.org, maniscalco+watch_chromium.org, haitaol+watch_chromium.org, extensions-reviews_chromium.org, pavely, maxbogue
Visibility:
Public.

Description

Introduce ProfileInvalidationProvider wrapper for InvalidationService TiclInvalidationService was refactored to no longer depend on Profile. However, it still is a KeyedService. While this does actually pull in Profile as a dependency, it implies that TiclInvalidationService should be considered a per-Profile service, like the other KeyedServices in Chrome's code base. This CL introduces ProfileInvalidationProvider, a KeyedService that is a tiny wrapper around an InvalidationService. Given this wrapper, InvalidationService (and with it, TiclInvalidationService) no longer have to be KeyedServices. BUG=362083 TEST=Updated unit and browser tests TBR=dcheng (chrome/browser/extensions/api/push_messaging/*) TBR=davemoore (chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc) TBR=arv (chrome/browser/ui/webui/invalidations_message_handler.cc) Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276715

Patch Set 1 #

Patch Set 2 : Fix test compilation on Android. #

Patch Set 3 : Fix clang compilation. #

Total comments: 4

Patch Set 4 : Addressed comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+497 lines, -532 lines) Patch
M chrome/browser/drive/DEPS View 1 2 3 1 chunk +1 line, -5 lines 0 comments Download
M chrome/browser/drive/drive_notification_manager.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/drive/drive_notification_manager_factory.cc View 1 2 3 3 chunks +10 lines, -7 lines 0 comments Download
M chrome/browser/extensions/api/push_messaging/push_messaging_api.cc View 1 2 3 7 chunks +30 lines, -24 lines 0 comments Download
M chrome/browser/extensions/api/push_messaging/push_messaging_apitest.cc View 1 2 3 5 chunks +19 lines, -6 lines 0 comments Download
M chrome/browser/invalidation/fake_invalidation_service.h View 2 chunks +0 lines, -6 lines 0 comments Download
M chrome/browser/invalidation/fake_invalidation_service.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M chrome/browser/invalidation/invalidation_service_android_unittest.cc View 1 4 chunks +4 lines, -9 lines 0 comments Download
D chrome/browser/invalidation/invalidation_service_factory.h View 1 chunk +0 lines, -66 lines 0 comments Download
D chrome/browser/invalidation/invalidation_service_factory.cc View 1 chunk +0 lines, -145 lines 0 comments Download
D chrome/browser/invalidation/invalidation_service_factory_browsertest.cc View 1 chunk +0 lines, -123 lines 0 comments Download
A chrome/browser/invalidation/profile_invalidation_provider_factory.h View 1 chunk +65 lines, -0 lines 0 comments Download
A + chrome/browser/invalidation/profile_invalidation_provider_factory.cc View 1 2 3 8 chunks +21 lines, -17 lines 0 comments Download
A chrome/browser/invalidation/profile_invalidation_provider_factory_browsertest.cc View 1 2 3 1 chunk +127 lines, -0 lines 0 comments Download
M chrome/browser/invalidation/ticl_invalidation_service.h View 2 chunks +1 line, -4 lines 0 comments Download
M chrome/browser/invalidation/ticl_invalidation_service.cc View 2 chunks +6 lines, -12 lines 0 comments Download
M chrome/browser/invalidation/ticl_invalidation_service_unittest.cc View 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/invalidation/ticl_profile_settings_provider_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/policy/cloud/cloud_policy_browsertest.cc View 1 2 3 4 chunks +20 lines, -5 lines 0 comments Download
M chrome/browser/policy/cloud/user_cloud_policy_invalidator.cc View 1 2 3 2 chunks +6 lines, -5 lines 0 comments Download
M chrome/browser/policy/cloud/user_cloud_policy_invalidator_factory.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/sync/glue/sync_backend_host_impl_unittest.cc View 1 2 3 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/sync/profile_sync_service.cc View 1 2 3 3 chunks +10 lines, -4 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_factory.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_typed_url_unittest.cc View 1 2 3 5 chunks +16 lines, -3 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_unittest.cc View 1 2 3 5 chunks +16 lines, -3 lines 0 comments Download
M chrome/browser/sync/test/integration/fake_server_invalidation_service.h View 2 chunks +0 lines, -6 lines 0 comments Download
M chrome/browser/sync/test/integration/fake_server_invalidation_service.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/sync/test/integration/sync_test.cc View 1 2 3 6 chunks +39 lines, -22 lines 0 comments Download
M chrome/browser/sync/test_profile_sync_service.cc View 1 2 3 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/invalidations_message_handler.cc View 1 2 3 3 chunks +14 lines, -11 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/chrome_tests.gypi View 2 chunks +2 lines, -2 lines 0 comments Download
M components/invalidation.gypi View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M components/invalidation/invalidation_service.h View 3 chunks +3 lines, -9 lines 0 comments Download
M components/invalidation/p2p_invalidation_service.h View 2 chunks +3 lines, -7 lines 0 comments Download
M components/invalidation/p2p_invalidation_service.cc View 1 chunk +0 lines, -4 lines 0 comments Download
A components/invalidation/profile_invalidation_provider.h View 1 2 3 1 chunk +37 lines, -0 lines 0 comments Download
A components/invalidation/profile_invalidation_provider.cc View 1 2 3 1 chunk +27 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
bartfab (slow)
Hi Richard, Could you please review: chrome/browser/invalidation/* componets/invalidation/* Hi Ryo, Could you please review chrome/browser/drive/DEPS
6 years, 6 months ago (2014-06-11 16:26:55 UTC) #1
rlarocque
+CC Pavel and Max. LGTM. I have some long-term concerns about this change. For example, ...
6 years, 6 months ago (2014-06-11 20:02:41 UTC) #2
rlarocque
On 2014/06/11 20:02:41, rlarocque wrote: > +CC Pavel and Max. > > LGTM. > > ...
6 years, 6 months ago (2014-06-11 20:03:56 UTC) #3
hashimoto
chrome/browser/drive lgtm https://codereview.chromium.org/327243003/diff/40001/chrome/browser/drive/drive_notification_manager_factory.cc File chrome/browser/drive/drive_notification_manager_factory.cc (right): https://codereview.chromium.org/327243003/diff/40001/chrome/browser/drive/drive_notification_manager_factory.cc#newcode61 chrome/browser/drive/drive_notification_manager_factory.cc:61: nit: Unneeded blank line.
6 years, 6 months ago (2014-06-12 02:20:46 UTC) #4
blundell
I don't really understand the motivations for this change. It's fine for a component to ...
6 years, 6 months ago (2014-06-12 07:45:37 UTC) #5
bartfab (slow)
On 2014/06/12 07:45:37, blundell wrote: > I don't really understand the motivations for this change. ...
6 years, 6 months ago (2014-06-12 08:19:41 UTC) #6
blundell
On 2014/06/12 08:19:41, bartfab wrote: > On 2014/06/12 07:45:37, blundell wrote: > > I don't ...
6 years, 6 months ago (2014-06-12 08:21:16 UTC) #7
bartfab (slow)
On 2014/06/12 08:21:16, blundell wrote: > On 2014/06/12 08:19:41, bartfab wrote: > > On 2014/06/12 ...
6 years, 6 months ago (2014-06-12 08:53:02 UTC) #8
blundell
On 2014/06/12 08:53:02, bartfab wrote: > On 2014/06/12 08:21:16, blundell wrote: > > On 2014/06/12 ...
6 years, 6 months ago (2014-06-12 08:55:15 UTC) #9
bartfab (slow)
On 2014/06/11 20:02:41, rlarocque wrote: > +CC Pavel and Max. > > LGTM. > > ...
6 years, 6 months ago (2014-06-12 09:21:08 UTC) #10
bartfab (slow)
https://codereview.chromium.org/327243003/diff/40001/chrome/browser/drive/drive_notification_manager_factory.cc File chrome/browser/drive/drive_notification_manager_factory.cc (right): https://codereview.chromium.org/327243003/diff/40001/chrome/browser/drive/drive_notification_manager_factory.cc#newcode61 chrome/browser/drive/drive_notification_manager_factory.cc:61: On 2014/06/12 02:20:46, hashimoto wrote: > nit: Unneeded blank ...
6 years, 6 months ago (2014-06-12 09:21:14 UTC) #11
bartfab (slow)
The CQ bit was checked by bartfab@chromium.org
6 years, 6 months ago (2014-06-12 09:21:27 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/bartfab@chromium.org/327243003/60001
6 years, 6 months ago (2014-06-12 09:24:38 UTC) #13
commit-bot: I haz the power
6 years, 6 months ago (2014-06-12 16:55:39 UTC) #14
Message was sent while issue was closed.
Change committed as 276715

Powered by Google App Engine
This is Rietveld 408576698