New issue
Advanced search Search tips

Issue 878446 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug

Blocking:
issue 801985



Sign in to add a comment

Refactor new invalidation implementation

Project Member Reported by melandory@chromium.org, Aug 28

Issue description

Refactor new invalidation implementation, so it doesn't depend on the primitives from the old one, e.g. ObjectId
 
Owner: melandory@chromium.org
Status: Assigned (was: Unconfirmed)
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 30

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

commit 381889a894e465272e02bf719b538f9a47d93c77
Author: Tanja Gornak <melandory@chromium.org>
Date: Thu Aug 30 11:18:29 2018

Refactoring. Deprecate InvalidatorRegistrar.

This CL deprectates the invalidator Registrar. The new
InvalidatorRegistrar will not use the ObjectId internally.

TBR=poromov@chromium.org

Bug: 878446, 801985
Change-Id: I0844401a01055e7e0365b45f485651d7dde45a49
Reviewed-on: https://chromium-review.googlesource.com/1193947
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587518}
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/chrome/browser/policy/cloud/remote_commands_invalidator_unittest.cc
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/chrome/browser/sync/test/integration/fake_server_invalidation_service.h
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/BUILD.gn
[rename] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/deprecated_invalidator_registrar.cc
[rename] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/deprecated_invalidator_registrar.h
[add] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/deprecated_invalidator_registrar_unittest.cc
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/fake_invalidation_service.h
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/fake_invalidator.h
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/fcm_invalidation_service.h
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/fcm_invalidator.h
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/invalidation_notifier.h
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/invalidation_service_android.h
[delete] https://crrev.com/480a30e22c991a3b3bb50e7d951d7502bbeccc97/components/invalidation/impl/invalidator_registrar_unittest.cc
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/non_blocking_invalidator.h
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/p2p_invalidator.h
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/ticl_invalidation_service.cc
[modify] https://crrev.com/381889a894e465272e02bf719b538f9a47d93c77/components/invalidation/impl/ticl_invalidation_service.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 30

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

commit 8eae0d61908feb7499ff9cfc38e6f3db46611c9f
Author: Tanja Gornak <melandory@chromium.org>
Date: Thu Aug 30 20:12:20 2018

[Tango->FCM] Refactoring. New InvalidatorRegistrar.

This CL creates new InvalidatorRegistrar.
New InvalidatorRegistrar depends on ObjectId as little as possible.

New InvalidatorRegistrar will be used on follow-up CL.
https://chromium-review.googlesource.com/c/chromium/src/+/1194032/5


Bug: 878446, 801985
Change-Id: I9f34d78a1eeb8d28a42ed0e90ee8701fba83192e
Reviewed-on: https://chromium-review.googlesource.com/1193887
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587728}
[modify] https://crrev.com/8eae0d61908feb7499ff9cfc38e6f3db46611c9f/components/invalidation/impl/BUILD.gn
[add] https://crrev.com/8eae0d61908feb7499ff9cfc38e6f3db46611c9f/components/invalidation/impl/invalidator_registrar.cc
[add] https://crrev.com/8eae0d61908feb7499ff9cfc38e6f3db46611c9f/components/invalidation/impl/invalidator_registrar.h
[add] https://crrev.com/8eae0d61908feb7499ff9cfc38e6f3db46611c9f/components/invalidation/impl/invalidator_registrar_unittest.cc
[modify] https://crrev.com/8eae0d61908feb7499ff9cfc38e6f3db46611c9f/components/invalidation/public/BUILD.gn
[modify] https://crrev.com/8eae0d61908feb7499ff9cfc38e6f3db46611c9f/components/invalidation/public/invalidation_util.h
[add] https://crrev.com/8eae0d61908feb7499ff9cfc38e6f3db46611c9f/components/invalidation/public/topic_invalidation_map.cc
[add] https://crrev.com/8eae0d61908feb7499ff9cfc38e6f3db46611c9f/components/invalidation/public/topic_invalidation_map.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 31

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

commit 366b7a569bf8257a878e0dd6404d6a2cb15df498
Author: Tanja Gornak <melandory@chromium.org>
Date: Fri Aug 31 01:40:01 2018

[Tango->FCM] Part 1. Ditch the objectId from the new FCM-based architecture.

ObjectId is the representation of the subscription topic from the
outdated Tango protocol.

It's not relevant to the new architecture, since subscription is represented
by topic name (in case of sync topic name is for example "BOOKMARKS" or
"PASSWORDS").

After this CL all code related to the FCMInvalidationService won't use the
ObjectId. But in some places the term Id still used. This is the subject for
follow up refactoring.


Bug: 878446, 801985
Change-Id: Idcf5bb464fff8976b319bd4721bdca2e3be77751
Reviewed-on: https://chromium-review.googlesource.com/1194032
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587904}
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/BUILD.gn
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/deprecated_invalidator_registrar_unittest.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fake_invalidator.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fake_invalidator.h
[add] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_fake_invalidator.cc
[add] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_fake_invalidator.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_invalidation_service.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_invalidation_service.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_invalidation_service_unittest.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_invalidator.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_invalidator.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_invalidator_unittest.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_sync_invalidation_listener.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_sync_invalidation_listener.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/fcm_sync_invalidation_listener_unittest.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/invalidation_logger.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/invalidation_logger.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/invalidation_notifier.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/invalidation_notifier.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/invalidator.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/invalidator_registrar_unittest.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/non_blocking_invalidator.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/non_blocking_invalidator.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/p2p_invalidator.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/p2p_invalidator.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/per_user_topic_registration_manager.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/per_user_topic_registration_manager.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/impl/per_user_topic_registration_manager_unittest.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/public/invalidation_util.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/public/invalidation_util.h
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/public/topic_invalidation_map.cc
[modify] https://crrev.com/366b7a569bf8257a878e0dd6404d6a2cb15df498/components/invalidation/public/topic_invalidation_map.h

Labels: Merge-Request-70
Requesting merge for the last commit in the bug
Labels: -Pri-3 Pri-1
Reason for merge -- this CL is pre-requirement for another CL, which was approved for the merge. I was sure, that it made it to the branch, but apparently not.
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 8

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 9

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e32e77b5ae0886cbf217da29322234e1a928ce58

commit e32e77b5ae0886cbf217da29322234e1a928ce58
Author: Tanja Gornak <melandory@chromium.org>
Date: Sun Sep 09 22:04:21 2018

[Tango->FCM] Part 1. Ditch the objectId from the new FCM-based architecture.

ObjectId is the representation of the subscription topic from the
outdated Tango protocol.

It's not relevant to the new architecture, since subscription is represented
by topic name (in case of sync topic name is for example "BOOKMARKS" or
"PASSWORDS").

After this CL all code related to the FCMInvalidationService won't use the
ObjectId. But in some places the term Id still used. This is the subject for
follow up refactoring.

TBR=melandory@chromium.org

(cherry picked from commit 366b7a569bf8257a878e0dd6404d6a2cb15df498)

Bug: 878446, 801985
Change-Id: Idcf5bb464fff8976b319bd4721bdca2e3be77751
Reviewed-on: https://chromium-review.googlesource.com/1194032
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#587904}
Reviewed-on: https://chromium-review.googlesource.com/1215203
Reviewed-by: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#193}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/BUILD.gn
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/deprecated_invalidator_registrar_unittest.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fake_invalidator.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fake_invalidator.h
[add] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_fake_invalidator.cc
[add] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_fake_invalidator.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_invalidation_service.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_invalidation_service.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_invalidation_service_unittest.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_invalidator.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_invalidator.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_invalidator_unittest.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_sync_invalidation_listener.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_sync_invalidation_listener.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/fcm_sync_invalidation_listener_unittest.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/invalidation_logger.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/invalidation_logger.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/invalidation_notifier.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/invalidation_notifier.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/invalidator.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/invalidator_registrar_unittest.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/non_blocking_invalidator.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/non_blocking_invalidator.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/p2p_invalidator.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/p2p_invalidator.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/per_user_topic_registration_manager.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/per_user_topic_registration_manager.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/impl/per_user_topic_registration_manager_unittest.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/public/invalidation_util.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/public/invalidation_util.h
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/public/topic_invalidation_map.cc
[modify] https://crrev.com/e32e77b5ae0886cbf217da29322234e1a928ce58/components/invalidation/public/topic_invalidation_map.h

Project Member

Comment 10 by bugdroid1@chromium.org, Yesterday (47 hours ago)

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

commit 204f7dd1e23e025335721f49566e845e0552e78a
Author: Tanja Gornak <melandory@chromium.org>
Date: Mon Jan 21 10:13:27 2019

[chrome://invalidations] Display SUBSCRIPTION_FAILURE status.

Bug: 801985, 878446
Change-Id: I62920b196afd6e8340dc955eb2e5afc7d5f638ec
Reviewed-on: https://chromium-review.googlesource.com/c/1412394
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624544}
[modify] https://crrev.com/204f7dd1e23e025335721f49566e845e0552e78a/components/invalidation/public/invalidator_state.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Today (23 hours ago)

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

commit f02a2b780c207e538eadda309f79de0b36db9f70
Author: Tanja Gornak <melandory@chromium.org>
Date: Tue Jan 22 09:56:00 2019

[Tango->FCM] Add unittests, which verify that failed subscription requests are retried

When the subscription request has failed, PerUserTopicRegistrationManager re-schedules
it. This CL adds unittests to verify that the request is rescheduled.

Bug: 801985, 878446
Change-Id: I98e5f34c7b83902a7bb5f63089a7405e81db5e8d
Reviewed-on: https://chromium-review.googlesource.com/c/1424841
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624751}
[modify] https://crrev.com/f02a2b780c207e538eadda309f79de0b36db9f70/components/invalidation/impl/per_user_topic_registration_manager.h
[modify] https://crrev.com/f02a2b780c207e538eadda309f79de0b36db9f70/components/invalidation/impl/per_user_topic_registration_manager_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Today (17 hours ago)

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

commit 89140af1a339c40bc2e1398939df41e3729d97c7
Author: Tanja Gornak <melandory@chromium.org>
Date: Tue Jan 22 16:38:53 2019

[Tango->FCM] Issue less change of status notifications.

Currently we notify the observers for every change in status
for the subscription channel. For example SUBSCRIPTION_FAILURE
is fired on every failed topic subscription. This is sub-optimal.
Instead we should notify the observers only on actual change.

Bug: 801985, 878446

Change-Id: I6f39caa3ec673c3a8e4f4c9aaa6a6cf69b1331a4
Reviewed-on: https://chromium-review.googlesource.com/c/1421177
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624797}
[modify] https://crrev.com/89140af1a339c40bc2e1398939df41e3729d97c7/components/invalidation/impl/per_user_topic_registration_manager.cc
[modify] https://crrev.com/89140af1a339c40bc2e1398939df41e3729d97c7/components/invalidation/impl/per_user_topic_registration_manager.h

Sign in to add a comment