New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 914882 link

Starred by 2 users
Status: Assigned
Owner:
mamir@chromium.org
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Clean up sync code for special handling of Mobile Bookmarks node

Project Member Reported by mamir@chromium.org, Dec 13 
In many places in the sync code (e.g. [1], [2]), including the new and shiny USS, there is special handling for Mobile Bookmarks node.

This is because the server doesn't always sync it down.

That decision is currently under evaluation on the server side.

This bug is to track the clean up effort for the client in case we change that logic in the client.

[1] https://cs.chromium.org/chromium/src/components/sync_bookmarks/bookmark_remote_updates_handler.cc?g=0&l=204

[2] https://cs.chromium.org/chromium/src/components/sync_bookmarks/bookmark_remote_updates_handler.cc?g=0&l=345
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 18 

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

commit 113dc1ec185ffded5be4c88929b5cf874e59225c
Author: Mikel Astiz <mastiz@chromium.org>
Date: Tue Dec 18 16:58:47 2018

Expect mobile bookmarks node from sync server

This affects USS bookmarks only: recent changes (i.e.
https://chromium-review.googlesource.com/c/1379751) guarantee that
the server's response will include the mobile bookmarks node.

This allows simplifying the client-side implementation substantially
and is believe to fix various CHECK/DCHECK failures related to
bookmarks not being tracked (e.g. in some corner cases a desktop
client may contain mobile bookmarks at the time sync is enabled).

Bug: 915198,914882
Change-Id: Ifb1b0dd7b4ffaca167b97f6a9279983b326b1872
Reviewed-on: https://chromium-review.googlesource.com/c/1380012
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617533}
[modify] https://crrev.com/113dc1ec185ffded5be4c88929b5cf874e59225c/components/sync_bookmarks/bookmark_model_observer_impl_unittest.cc
[modify] https://crrev.com/113dc1ec185ffded5be4c88929b5cf874e59225c/components/sync_bookmarks/bookmark_model_type_processor.cc
[modify] https://crrev.com/113dc1ec185ffded5be4c88929b5cf874e59225c/components/sync_bookmarks/bookmark_model_type_processor_unittest.cc
[modify] https://crrev.com/113dc1ec185ffded5be4c88929b5cf874e59225c/components/sync_bookmarks/bookmark_remote_updates_handler.cc
[modify] https://crrev.com/113dc1ec185ffded5be4c88929b5cf874e59225c/components/sync_bookmarks/bookmark_remote_updates_handler_unittest.cc
[modify] https://crrev.com/113dc1ec185ffded5be4c88929b5cf874e59225c/components/sync_bookmarks/synced_bookmark_tracker.cc
[modify] https://crrev.com/113dc1ec185ffded5be4c88929b5cf874e59225c/components/sync_bookmarks/synced_bookmark_tracker_unittest.cc
►

Sign in to add a comment