New issue
Advanced search Search tips

Issue 879571 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug

Blocking:
issue 801985



Sign in to add a comment

Pass message from the FCM channel to the invalidation Listener

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

Issue description

* Receive message from the FCM channel.
* Extract data from the message.
* Pass parsed message to the Listener.
* Convert parsed message to the invalidation.



 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 3

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

commit 5944dfa28a03929d611d13e7c02ed274bf10ae28
Author: Tanja Gornak <melandory@chromium.org>
Date: Mon Sep 03 14:31:40 2018

[Tango->FCM]  Parse incoming message to the invalidation.

This CL implements following logic:
* Receive message from the FCM channel.
* Extract data from the message.
* Pass parsed message to the Listener.
* Convert parsed message to the invalidation.

The message receiving logic should gracefully handle these two
important corner cases.

(1) Subscription race. The client sends registration request for the
topic T1. But before response arrives invalidation for the topic T1
arrives. Such invalidations shouldn't be dropped.
(2) Unsubscription race. Client send unsubscription request for topic T1.
The invalidations for the topic T1 should be immediately dropped.



Bug:  879571 , 801985
Change-Id: I79fa620122b9df54203c27f09dd2569bdcdb4c30
Reviewed-on: https://chromium-review.googlesource.com/1193884
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588390}
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/chrome/browser/sync/profile_sync_service_factory.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/fcm_network_handler.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/fcm_network_handler_unittests.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/fcm_sync_invalidation_listener.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/fcm_sync_invalidation_listener.h
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/fcm_sync_invalidation_listener_unittest.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/fcm_sync_network_channel.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/fcm_sync_network_channel.h
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/invalidation_listener.h
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/network_channel.h
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/per_user_topic_invalidation_client.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/per_user_topic_invalidation_client.h
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/per_user_topic_invalidation_client_unittest.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/per_user_topic_registration_manager.cc
[modify] https://crrev.com/5944dfa28a03929d611d13e7c02ed274bf10ae28/components/invalidation/impl/per_user_topic_registration_manager.h

Comment 2 Deleted

Labels: OS-Linux OS-Mac OS-Windows
Labels: -Pri-3 Pri-1
Labels: -Merge-Request-70 Merge-Request-68
Labels: -Merge-Request-68 Merge-Request-70
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 4

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

commit 2a6dd62b8ff79d9271b0b71289eabb61e1b44c9e
Author: Tanja Gornak <melandory@chromium.org>
Date: Tue Sep 04 16:37:49 2018

[Tango->FCM] Remove static initializer from invalidation client

Bug:  879571 , 801985,  880344 
TBR=pavely@chromium.org

Change-Id: I224e704c348d031e68e7341122331015d20e9f61
Reviewed-on: https://chromium-review.googlesource.com/1204017
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588543}
[modify] https://crrev.com/2a6dd62b8ff79d9271b0b71289eabb61e1b44c9e/components/invalidation/impl/per_user_topic_invalidation_client.cc

Project Member

Comment 8 by sheriffbot@chromium.org, Sep 5

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 9 by sheriffbot@chromium.org, Sep 10

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 10 by bugdroid1@chromium.org, Sep 11

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

commit d91043bc6fb9b85e122986d2b5edbfaf8ff07166
Author: Tanja Gornak <melandory@chromium.org>
Date: Tue Sep 11 22:51:51 2018

[Tango->FCM]  Parse incoming message to the invalidation.

This CL implements following logic:
* Receive message from the FCM channel.
* Extract data from the message.
* Pass parsed message to the Listener.
* Convert parsed message to the invalidation.

The message receiving logic should gracefully handle these two
important corner cases.

(1) Subscription race. The client sends registration request for the
topic T1. But before response arrives invalidation for the topic T1
arrives. Such invalidations shouldn't be dropped.
(2) Unsubscription race. Client send unsubscription request for topic T1.
The invalidations for the topic T1 should be immediately dropped.



TBR=melandory@chromium.org

(cherry picked from commit 5944dfa28a03929d611d13e7c02ed274bf10ae28)

Bug:  879571 , 801985
Change-Id: I79fa620122b9df54203c27f09dd2569bdcdb4c30
Reviewed-on: https://chromium-review.googlesource.com/1193884
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588390}
Reviewed-on: https://chromium-review.googlesource.com/1220746
Reviewed-by: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#302}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/chrome/browser/sync/profile_sync_service_factory.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/fcm_network_handler.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/fcm_network_handler_unittests.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/fcm_sync_invalidation_listener.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/fcm_sync_invalidation_listener.h
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/fcm_sync_invalidation_listener_unittest.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/fcm_sync_network_channel.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/fcm_sync_network_channel.h
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/invalidation_listener.h
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/network_channel.h
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/per_user_topic_invalidation_client.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/per_user_topic_invalidation_client.h
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/per_user_topic_invalidation_client_unittest.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/per_user_topic_registration_manager.cc
[modify] https://crrev.com/d91043bc6fb9b85e122986d2b5edbfaf8ff07166/components/invalidation/impl/per_user_topic_registration_manager.h

Project Member

Comment 11 by bugdroid1@chromium.org, Sep 11

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

commit 1a627cd0a1ee2e6e1d756eeb1c92eeeed15d3d9e
Author: Tanja Gornak <melandory@chromium.org>
Date: Tue Sep 11 22:55:08 2018

[Tango->FCM] Remove static initializer from invalidation client

Bug:  879571 , 801985,  880344 
TBR=melandory@chromium.org, pavely@chromium.org

(cherry picked from commit 2a6dd62b8ff79d9271b0b71289eabb61e1b44c9e)

Change-Id: I224e704c348d031e68e7341122331015d20e9f61
Reviewed-on: https://chromium-review.googlesource.com/1204017
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Tatiana Gornak <melandory@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588543}
Reviewed-on: https://chromium-review.googlesource.com/1220148
Cr-Commit-Position: refs/branch-heads/3538@{#303}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/1a627cd0a1ee2e6e1d756eeb1c92eeeed15d3d9e/components/invalidation/impl/per_user_topic_invalidation_client.cc

Status: Fixed (was: Assigned)

Sign in to add a comment