Enabling FCMInvalidations breaks Drive push messaging |
||||||||||||||||||||
Issue descriptionDrive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. ProfileSyncService only calls SetActiveAccountId on the IdentityProvider from the ProfileInvalidationProvider implementation it uses. If FCMInvalidations is enabled, the IdentityProvider used by DeprecatedProfileInvalidationProviderFactory is never initialized with the active account ID so never starts. ProfileSyncService should call SetActiveAccountId on both IdentityProvider instances as long as DeprecatedProfileInvalidationProviderFactory is used.
,
Oct 23
,
Oct 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4 commit e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4 Author: Tanja Gornak <melandory@chromium.org> Date: Sat Oct 27 11:11:03 2018 [Tango->FCM] Fix the Drive push messaging. Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. ProfileSyncService only calls SetActiveAccountId on the IdentityProvider from the ProfileInvalidationProvider implementation it uses. If FCMInvalidations is enabled, the IdentityProvider used by DeprecatedProfileInvalidationProviderFactory is never initialized with the active account ID so never starts. ProfileSyncService should call SetActiveAccountId on both IdentityProvider instances as long as DeprecatedProfileInvalidationProviderFactory is used. Bug: 897570 Change-Id: I5d452a7aa4e0d3fb8496a213495eafef9e5a0351 Reviewed-on: https://chromium-review.googlesource.com/c/1296494 Commit-Queue: Tatiana Gornak <melandory@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: John Wu <jzw@chromium.org> Cr-Commit-Position: refs/heads/master@{#603328} [modify] https://crrev.com/e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4/chrome/browser/sync/profile_sync_service_factory.cc [modify] https://crrev.com/e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4/chrome/browser/sync/profile_sync_test_util.cc [modify] https://crrev.com/e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4/components/browser_sync/profile_sync_service.cc [modify] https://crrev.com/e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4/components/browser_sync/profile_sync_service.h [modify] https://crrev.com/e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4/components/browser_sync/profile_sync_test_util.cc [modify] https://crrev.com/e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4/ios/chrome/browser/sync/profile_sync_service_factory.cc [modify] https://crrev.com/e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4/ios/web_view/internal/sync/web_view_profile_sync_service_factory.mm
,
Oct 29
,
Oct 29
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 29
Before we approve merge to M71, please answer followings: * Is this M71 regression? Is it critical? * Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M71? * Any other important details to justify the merge. Please note M71 is already in Beta, so merge bar is very high. Thank you.
,
Oct 30
> Is this M71 regression? Is it critical? Yes > Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M71? Yep > Any other important details to justify the merge. The bug breaks the drive notification feature.
,
Oct 30
Approving merge to M71 branch 3578 based on comment #7. Pls merge ASAP. Thank you.
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/88b4f3847f14e29b475379b45ee302fe50524d41 commit 88b4f3847f14e29b475379b45ee302fe50524d41 Author: Tanja Gornak <melandory@chromium.org> Date: Tue Oct 30 18:26:32 2018 [Tango->FCM] Fix the Drive push messaging. Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. ProfileSyncService only calls SetActiveAccountId on the IdentityProvider from the ProfileInvalidationProvider implementation it uses. If FCMInvalidations is enabled, the IdentityProvider used by DeprecatedProfileInvalidationProviderFactory is never initialized with the active account ID so never starts. ProfileSyncService should call SetActiveAccountId on both IdentityProvider instances as long as DeprecatedProfileInvalidationProviderFactory is used. TBR=melandory@chromium.org (cherry picked from commit e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4) Bug: 897570 Change-Id: I5d452a7aa4e0d3fb8496a213495eafef9e5a0351 Reviewed-on: https://chromium-review.googlesource.com/c/1296494 Commit-Queue: Tatiana Gornak <melandory@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: John Wu <jzw@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#603328} Reviewed-on: https://chromium-review.googlesource.com/c/1308293 Reviewed-by: Tatiana Gornak <melandory@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#408} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/88b4f3847f14e29b475379b45ee302fe50524d41/chrome/browser/sync/profile_sync_service_factory.cc [modify] https://crrev.com/88b4f3847f14e29b475379b45ee302fe50524d41/chrome/browser/sync/profile_sync_test_util.cc [modify] https://crrev.com/88b4f3847f14e29b475379b45ee302fe50524d41/components/browser_sync/profile_sync_service.cc [modify] https://crrev.com/88b4f3847f14e29b475379b45ee302fe50524d41/components/browser_sync/profile_sync_service.h [modify] https://crrev.com/88b4f3847f14e29b475379b45ee302fe50524d41/components/browser_sync/profile_sync_test_util.cc
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/88b4f3847f14e29b475379b45ee302fe50524d41 Commit: 88b4f3847f14e29b475379b45ee302fe50524d41 Author: melandory@chromium.org Commiter: melandory@chromium.org Date: 2018-10-30 18:26:32 +0000 UTC [Tango->FCM] Fix the Drive push messaging. Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. ProfileSyncService only calls SetActiveAccountId on the IdentityProvider from the ProfileInvalidationProvider implementation it uses. If FCMInvalidations is enabled, the IdentityProvider used by DeprecatedProfileInvalidationProviderFactory is never initialized with the active account ID so never starts. ProfileSyncService should call SetActiveAccountId on both IdentityProvider instances as long as DeprecatedProfileInvalidationProviderFactory is used. TBR=melandory@chromium.org (cherry picked from commit e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4) Bug: 897570 Change-Id: I5d452a7aa4e0d3fb8496a213495eafef9e5a0351 Reviewed-on: https://chromium-review.googlesource.com/c/1296494 Commit-Queue: Tatiana Gornak <melandory@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: John Wu <jzw@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#603328} Reviewed-on: https://chromium-review.googlesource.com/c/1308293 Reviewed-by: Tatiana Gornak <melandory@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#408} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 5
,
Nov 8
,
Nov 8
,
Nov 9
,
Nov 9
Hey, Kariah. I just realized that it's possible to merge this change for the ios too and it will fix chrome sync issue I've outlined in the email thread yesterday. This CL is already in canary
,
Nov 9
This bug requires manual review: Less than 21 days to go before AppStore submit on M71 Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 9
,
Nov 9
Since the fix is on canary already, can we get verification that it's working there please?
,
Nov 9
I have verified myself that the change works on canary.
,
Nov 9
Approved.
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8cae3d68f0556ee145341afe4c2b7118f59092c9 Commit: 8cae3d68f0556ee145341afe4c2b7118f59092c9 Author: melandory@chromium.org Commiter: melandory@chromium.org Date: 2018-11-12 10:58:34 +0000 UTC [Tango->FCM] Fix the Drive push messaging. Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. ProfileSyncService only calls SetActiveAccountId on the IdentityProvider from the ProfileInvalidationProvider implementation it uses. If FCMInvalidations is enabled, the IdentityProvider used by DeprecatedProfileInvalidationProviderFactory is never initialized with the active account ID so never starts. ProfileSyncService should call SetActiveAccountId on both IdentityProvider instances as long as DeprecatedProfileInvalidationProviderFactory is used. TBR=melandory@chromium.org, treib@chromium.org, jzw@chromium.org Bug: 897570 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: Ie3dd272e90f490199a62f83c3837f0f1a5256784 Reviewed-on: https://chromium-review.googlesource.com/c/1296494 Commit-Queue: Tatiana Gornak <melandory@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: John Wu <jzw@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/1329680 Reviewed-by: Tatiana Gornak <melandory@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#632} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8cae3d68f0556ee145341afe4c2b7118f59092c9 commit 8cae3d68f0556ee145341afe4c2b7118f59092c9 Author: Tanja Gornak <melandory@chromium.org> Date: Mon Nov 12 10:58:34 2018 [Tango->FCM] Fix the Drive push messaging. Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. ProfileSyncService only calls SetActiveAccountId on the IdentityProvider from the ProfileInvalidationProvider implementation it uses. If FCMInvalidations is enabled, the IdentityProvider used by DeprecatedProfileInvalidationProviderFactory is never initialized with the active account ID so never starts. ProfileSyncService should call SetActiveAccountId on both IdentityProvider instances as long as DeprecatedProfileInvalidationProviderFactory is used. TBR=melandory@chromium.org, treib@chromium.org, jzw@chromium.org Bug: 897570 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: Ie3dd272e90f490199a62f83c3837f0f1a5256784 Reviewed-on: https://chromium-review.googlesource.com/c/1296494 Commit-Queue: Tatiana Gornak <melandory@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: John Wu <jzw@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/1329680 Reviewed-by: Tatiana Gornak <melandory@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#632} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/8cae3d68f0556ee145341afe4c2b7118f59092c9/ios/chrome/browser/sync/profile_sync_service_factory.cc [modify] https://crrev.com/8cae3d68f0556ee145341afe4c2b7118f59092c9/ios/web_view/internal/sync/web_view_profile_sync_service_factory.mm
,
Nov 13
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f486af29db30399182244b5d22dbe177bd790ee6 commit f486af29db30399182244b5d22dbe177bd790ee6 Author: Olivier Robin <olivierrobin@chromium.org> Date: Tue Nov 13 09:55:36 2018 Revert "[Tango->FCM] Fix the Drive push messaging." This reverts commit 8cae3d68f0556ee145341afe4c2b7118f59092c9. Reason for revert: Breaks beta compilation ../../ios/web_view/internal/sync/web_view_profile_sync_service_factory.mm:98:11: error: member access into incomplete type 'invalidation::ProfileInvalidationProvider' ->GetIdentityProvider()); Original change's description: > [Tango->FCM] Fix the Drive push messaging. > > Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. > ProfileSyncService only calls SetActiveAccountId on the IdentityProvider > from the ProfileInvalidationProvider implementation it uses. > > If FCMInvalidations is enabled, the IdentityProvider used by > DeprecatedProfileInvalidationProviderFactory is never initialized > with the active account ID so never starts. > > ProfileSyncService should call SetActiveAccountId on both IdentityProvider > instances as long as DeprecatedProfileInvalidationProviderFactory is used. > > TBR=melandory@chromium.org, treib@chromium.org, jzw@chromium.org > > Bug: 897570 > Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs > Change-Id: Ie3dd272e90f490199a62f83c3837f0f1a5256784 > Reviewed-on: https://chromium-review.googlesource.com/c/1296494 > Commit-Queue: Tatiana Gornak <melandory@chromium.org> > Reviewed-by: Marc Treib <treib@chromium.org> > Reviewed-by: John Wu <jzw@chromium.org> > Reviewed-on: https://chromium-review.googlesource.com/c/1329680 > Reviewed-by: Tatiana Gornak <melandory@chromium.org> > Cr-Commit-Position: refs/branch-heads/3578@{#632} > Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} TBR=treib@chromium.org,melandory@chromium.org,jzw@chromium.org Change-Id: I12be7e1df54b51e70b8a08f85430293d51e7cce9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 897570 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Reviewed-on: https://chromium-review.googlesource.com/c/1332933 Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#653} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/f486af29db30399182244b5d22dbe177bd790ee6/ios/chrome/browser/sync/profile_sync_service_factory.cc [modify] https://crrev.com/f486af29db30399182244b5d22dbe177bd790ee6/ios/web_view/internal/sync/web_view_profile_sync_service_factory.mm
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f486af29db30399182244b5d22dbe177bd790ee6 Commit: f486af29db30399182244b5d22dbe177bd790ee6 Author: olivierrobin@chromium.org Commiter: olivierrobin@chromium.org Date: 2018-11-13 09:55:36 +0000 UTC Revert "[Tango->FCM] Fix the Drive push messaging." This reverts commit 8cae3d68f0556ee145341afe4c2b7118f59092c9. Reason for revert: Breaks beta compilation ../../ios/web_view/internal/sync/web_view_profile_sync_service_factory.mm:98:11: error: member access into incomplete type 'invalidation::ProfileInvalidationProvider' ->GetIdentityProvider()); Original change's description: > [Tango->FCM] Fix the Drive push messaging. > > Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. > ProfileSyncService only calls SetActiveAccountId on the IdentityProvider > from the ProfileInvalidationProvider implementation it uses. > > If FCMInvalidations is enabled, the IdentityProvider used by > DeprecatedProfileInvalidationProviderFactory is never initialized > with the active account ID so never starts. > > ProfileSyncService should call SetActiveAccountId on both IdentityProvider > instances as long as DeprecatedProfileInvalidationProviderFactory is used. > > TBR=melandory@chromium.org, treib@chromium.org, jzw@chromium.org > > Bug: 897570 > Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs > Change-Id: Ie3dd272e90f490199a62f83c3837f0f1a5256784 > Reviewed-on: https://chromium-review.googlesource.com/c/1296494 > Commit-Queue: Tatiana Gornak <melandory@chromium.org> > Reviewed-by: Marc Treib <treib@chromium.org> > Reviewed-by: John Wu <jzw@chromium.org> > Reviewed-on: https://chromium-review.googlesource.com/c/1329680 > Reviewed-by: Tatiana Gornak <melandory@chromium.org> > Cr-Commit-Position: refs/branch-heads/3578@{#632} > Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} TBR=treib@chromium.org,melandory@chromium.org,jzw@chromium.org Change-Id: I12be7e1df54b51e70b8a08f85430293d51e7cce9 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 897570 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Reviewed-on: https://chromium-review.googlesource.com/c/1332933 Reviewed-by: Olivier Robin <olivierrobin@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#653} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a941b49c7238ce71f5c4a6de6b4dc0247540f15e commit a941b49c7238ce71f5c4a6de6b4dc0247540f15e Author: Tanja Gornak <melandory@chromium.org> Date: Tue Nov 13 11:13:07 2018 [Tango->FCM] Fix the Drive push messaging. Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. ProfileSyncService only calls SetActiveAccountId on the IdentityProvider from the ProfileInvalidationProvider implementation it uses. If FCMInvalidations is enabled, the IdentityProvider used by DeprecatedProfileInvalidationProviderFactory is never initialized with the active account ID so never starts. ProfileSyncService should call SetActiveAccountId on both IdentityProvider instances as long as DeprecatedProfileInvalidationProviderFactory is used. TBR=treib@chromium.org, jzw@chromium.org (cherry picked from commit e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4) Bug: 897570 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: I342b713ad0e05d0683a68ba39625c06d7afa4aad Reviewed-on: https://chromium-review.googlesource.com/c/1296494 Commit-Queue: Tatiana Gornak <melandory@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: John Wu <jzw@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#603328} Reviewed-on: https://chromium-review.googlesource.com/c/1333371 Reviewed-by: Tatiana Gornak <melandory@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#656} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/a941b49c7238ce71f5c4a6de6b4dc0247540f15e/ios/chrome/browser/sync/profile_sync_service_factory.cc
,
Nov 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a941b49c7238ce71f5c4a6de6b4dc0247540f15e Commit: a941b49c7238ce71f5c4a6de6b4dc0247540f15e Author: melandory@chromium.org Commiter: melandory@chromium.org Date: 2018-11-13 11:13:07 +0000 UTC [Tango->FCM] Fix the Drive push messaging. Drive relies on DeprecatedProfileInvalidationProviderFactory for its invalidations. ProfileSyncService only calls SetActiveAccountId on the IdentityProvider from the ProfileInvalidationProvider implementation it uses. If FCMInvalidations is enabled, the IdentityProvider used by DeprecatedProfileInvalidationProviderFactory is never initialized with the active account ID so never starts. ProfileSyncService should call SetActiveAccountId on both IdentityProvider instances as long as DeprecatedProfileInvalidationProviderFactory is used. TBR=treib@chromium.org, jzw@chromium.org (cherry picked from commit e0e4c2bd0d3be3166c55832cb248cb33b2d45fb4) Bug: 897570 Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs Change-Id: I342b713ad0e05d0683a68ba39625c06d7afa4aad Reviewed-on: https://chromium-review.googlesource.com/c/1296494 Commit-Queue: Tatiana Gornak <melandory@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: John Wu <jzw@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#603328} Reviewed-on: https://chromium-review.googlesource.com/c/1333371 Reviewed-by: Tatiana Gornak <melandory@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#656} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Dec 5
|
||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||
Comment 1 by melandory@chromium.org
, Oct 23