Listen to XMPP notifications for team drives |
|||||||||
Issue descriptionFrom manual testing the XMPP notifications seem to always be disabled, need to investigate why. There is a timer that causes a sync to occur in the absence of XMPP notifications.
,
Jul 25
XMPP notifications for the default corpus seem OK, we need them for team drives tho.
,
Jul 25
,
Aug 9
,
Aug 30
,
Aug 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/664e2296f298a6a23804f41dec73bab472949787 commit 664e2296f298a6a23804f41dec73bab472949787 Author: Stuart Langley <slangley@google.com> Date: Fri Aug 31 04:58:28 2018 Add XMPP push notifications for changes to team drives. Update DriveNotificationManager so that it can register for, and process, XMPP push notifications for team drives. We can register for changes in COSMO by prefixing "TD:" to the team drive ID when we register for the invalidation. To support this change we: - Add OnTeamDrivesUpdated to FileSystemObserver to flow changes in the team drives that the user can access to observers. - Allow DriveNotificationManager to keep track of the users team drive ID's and register for notifications from them. - Change DriveNotificationManager to generate a different event if the update is triggered by the polling timer or a push notification. - Change CheckForUpdates to accept a set of id's which will be the team drive id's that were recieved from the invalidation service. - Provide a glue class in DriveIntegrationService to pass team drive updates to DriveNotificationManager. - Addes file system test cases. Follow ups: - Fix DriveNotificationManager so we can write unit tests against it. Write the tests for registering for and processing invalidations. - Look at batching in DriveNotifcationManager, as invalidation service can send many rapid invalidations for the same object. - Fix up the polling timer logic in DriveNotificationManager, and adjust the timeouts so we do not spam the drive backends with needless update checks if the user has many team drives. Bug: 837456 , 783184 , 867166 Change-Id: I3fd80d41902bdd7f6c8f090346acbbf64f9dafdb Reviewed-on: https://chromium-review.googlesource.com/1195256 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: Stuart Langley <slangley@chromium.org> Cr-Commit-Position: refs/heads/master@{#587945} [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/chrome/browser/chromeos/drive/drive_integration_service.cc [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/chrome/browser/chromeos/drive/drive_integration_service.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/chrome/browser/extensions/api/sync_file_system/sync_file_system_browsertest.cc [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/chrome/browser/sync_file_system/drive_backend/sync_engine.cc [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/chrome/browser/sync_file_system/drive_backend/sync_engine.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/chromeos/dummy_file_system.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/chromeos/fake_file_system.cc [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/chromeos/fake_file_system.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/chromeos/file_system.cc [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/chromeos/file_system.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/chromeos/file_system_interface.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/chromeos/file_system_observer.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/drive_notification_manager.cc [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/drive_notification_manager.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/drive_notification_observer.h [modify] https://crrev.com/664e2296f298a6a23804f41dec73bab472949787/components/drive/file_system_unittest.cc
,
Sep 5
Landed https://chromium-review.googlesource.com/c/chromium/src/+/1198712, but the bot has not updated the bug. Requesting merge for M-70.
,
Sep 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80845865e5f2f5a04d1222055275a1a351e68347 commit 80845865e5f2f5a04d1222055275a1a351e68347 Author: Stuart Langley <slangley@google.com> Date: Wed Sep 05 00:47:29 2018 Add batching for XMPP notifications. The invalidation service many send many rapid invalidations for the same team drive or the users default changelist in succession. We only want to run the changelist updater once to pick up the latest changes, so rather than just calling CheckForUpdates() for every invalidation we can batch them together by waiting for a short period before calling CheckForUpdates. Also, introduce the opertion queue for these updates that same as we use for polling updates, as the updates occur in the background and we do not want to flood the request queue and make the app un-responsive. Add tests for DriveNotificationManager to cover my recent changes in this area, as a followup I'll add more test to cover the logic that existed previously. Bug: 837456 Change-Id: I99bf023178fa364f4f9b17799de9f086b15c05d3 Reviewed-on: https://chromium-review.googlesource.com/1198712 Commit-Queue: Stuart Langley <slangley@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Cr-Commit-Position: refs/heads/master@{#588720} [modify] https://crrev.com/80845865e5f2f5a04d1222055275a1a351e68347/chrome/browser/chromeos/BUILD.gn [modify] https://crrev.com/80845865e5f2f5a04d1222055275a1a351e68347/components/drive/chromeos/file_system.cc [modify] https://crrev.com/80845865e5f2f5a04d1222055275a1a351e68347/components/drive/drive_notification_manager.cc [modify] https://crrev.com/80845865e5f2f5a04d1222055275a1a351e68347/components/drive/drive_notification_manager.h [add] https://crrev.com/80845865e5f2f5a04d1222055275a1a351e68347/components/drive/drive_notification_manager_unittest.cc
,
Sep 6
,
Sep 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f6e73b0a19d3647b5af01354ac684f79711c397d commit f6e73b0a19d3647b5af01354ac684f79711c397d Author: Stuart Langley <slangley@google.com> Date: Thu Sep 06 23:34:57 2018 Add XMPP push notifications for changes to team drives. Update DriveNotificationManager so that it can register for, and process, XMPP push notifications for team drives. We can register for changes in COSMO by prefixing "TD:" to the team drive ID when we register for the invalidation. To support this change we: - Add OnTeamDrivesUpdated to FileSystemObserver to flow changes in the team drives that the user can access to observers. - Allow DriveNotificationManager to keep track of the users team drive ID's and register for notifications from them. - Change DriveNotificationManager to generate a different event if the update is triggered by the polling timer or a push notification. - Change CheckForUpdates to accept a set of id's which will be the team drive id's that were recieved from the invalidation service. - Provide a glue class in DriveIntegrationService to pass team drive updates to DriveNotificationManager. - Addes file system test cases. Follow ups: - Fix DriveNotificationManager so we can write unit tests against it. Write the tests for registering for and processing invalidations. - Look at batching in DriveNotifcationManager, as invalidation service can send many rapid invalidations for the same object. - Fix up the polling timer logic in DriveNotificationManager, and adjust the timeouts so we do not spam the drive backends with needless update checks if the user has many team drives. Bug: 837456 , 783184 , 867166 Change-Id: I3fd80d41902bdd7f6c8f090346acbbf64f9dafdb Reviewed-on: https://chromium-review.googlesource.com/1195256 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: Stuart Langley <slangley@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#587945}(cherry picked from commit 664e2296f298a6a23804f41dec73bab472949787) Reviewed-on: https://chromium-review.googlesource.com/1212102 Reviewed-by: Stuart Langley <slangley@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#117} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/chrome/browser/chromeos/drive/drive_integration_service.cc [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/chrome/browser/chromeos/drive/drive_integration_service.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/chrome/browser/extensions/api/sync_file_system/sync_file_system_browsertest.cc [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/chrome/browser/sync_file_system/drive_backend/drive_backend_sync_unittest.cc [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/chrome/browser/sync_file_system/drive_backend/sync_engine.cc [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/chrome/browser/sync_file_system/drive_backend/sync_engine.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/chromeos/dummy_file_system.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/chromeos/fake_file_system.cc [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/chromeos/fake_file_system.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/chromeos/file_system.cc [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/chromeos/file_system.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/chromeos/file_system_interface.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/chromeos/file_system_observer.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/drive_notification_manager.cc [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/drive_notification_manager.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/drive_notification_observer.h [modify] https://crrev.com/f6e73b0a19d3647b5af01354ac684f79711c397d/components/drive/file_system_unittest.cc
,
Sep 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fb0a9c019978e83d293462ab0236aa153c52b047 commit fb0a9c019978e83d293462ab0236aa153c52b047 Author: Stuart Langley <slangley@google.com> Date: Thu Sep 06 23:35:57 2018 Add batching for XMPP notifications. The invalidation service many send many rapid invalidations for the same team drive or the users default changelist in succession. We only want to run the changelist updater once to pick up the latest changes, so rather than just calling CheckForUpdates() for every invalidation we can batch them together by waiting for a short period before calling CheckForUpdates. Also, introduce the opertion queue for these updates that same as we use for polling updates, as the updates occur in the background and we do not want to flood the request queue and make the app un-responsive. Add tests for DriveNotificationManager to cover my recent changes in this area, as a followup I'll add more test to cover the logic that existed previously. Bug: 837456 Change-Id: I99bf023178fa364f4f9b17799de9f086b15c05d3 Reviewed-on: https://chromium-review.googlesource.com/1198712 Commit-Queue: Stuart Langley <slangley@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#588720}(cherry picked from commit 80845865e5f2f5a04d1222055275a1a351e68347) Reviewed-on: https://chromium-review.googlesource.com/1212122 Reviewed-by: Stuart Langley <slangley@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#118} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/fb0a9c019978e83d293462ab0236aa153c52b047/chrome/browser/chromeos/BUILD.gn [modify] https://crrev.com/fb0a9c019978e83d293462ab0236aa153c52b047/components/drive/chromeos/file_system.cc [modify] https://crrev.com/fb0a9c019978e83d293462ab0236aa153c52b047/components/drive/drive_notification_manager.cc [modify] https://crrev.com/fb0a9c019978e83d293462ab0236aa153c52b047/components/drive/drive_notification_manager.h [add] https://crrev.com/fb0a9c019978e83d293462ab0236aa153c52b047/components/drive/drive_notification_manager_unittest.cc
,
Sep 6
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by slangley@chromium.org
, Jun 21 2018Status: Assigned (was: Available)