Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(502)

Issue 10828268: [Sync] Fix mobile bookmark folder association on non-android (Closed)

Created:
8 years, 4 months ago by Nicolas Zea
Modified:
8 years, 4 months ago
CC:
chromium-reviews, Raghu Simha, ncarter (slow), akalin
Visibility:
Public.

Description

[Sync] Fix mobile bookmark folder association on non-android We were failing to associate the mobile bookmarks folder at sync startup time. This could result in various seemingly-random bookmark unrecoverable errors. We now associate the bookmark folder if it's there, and condition triggering an association error on whether we expected it to be there (which we only do on ANDROID platforms). BUG=121587 TEST=Sign in to sync with a clean profile, but synced mobile bookmarks existing in the mobile bookmarks folder on the server. Bookmarks should appear on the desktop client. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151178

Patch Set 1 #

Patch Set 2 : Add ifdef #

Total comments: 1

Patch Set 3 : Review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -12 lines) Patch
M chrome/browser/sync/glue/bookmark_model_associator.cc View 1 2 1 chunk +7 lines, -11 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_bookmark_unittest.cc View 1 2 chunks +7 lines, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
Nicolas Zea
PTAL
8 years, 4 months ago (2012-08-10 23:26:08 UTC) #1
tim (not reviewing)
http://codereview.chromium.org/10828268/diff/6001/chrome/browser/sync/glue/bookmark_model_associator.cc File chrome/browser/sync/glue/bookmark_model_associator.cc (right): http://codereview.chromium.org/10828268/diff/6001/chrome/browser/sync/glue/bookmark_model_associator.cc#newcode412 chrome/browser/sync/glue/bookmark_model_associator.cc:412: else The 'else' isn't really necessary here. Lets just ...
8 years, 4 months ago (2012-08-10 23:31:22 UTC) #2
tim (not reviewing)
On 2012/08/10 23:31:22, timsteele wrote: > http://codereview.chromium.org/10828268/diff/6001/chrome/browser/sync/glue/bookmark_model_associator.cc > File chrome/browser/sync/glue/bookmark_model_associator.cc (right): > > http://codereview.chromium.org/10828268/diff/6001/chrome/browser/sync/glue/bookmark_model_associator.cc#newcode412 > ...
8 years, 4 months ago (2012-08-10 23:31:31 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zea@chromium.org/10828268/10001
8 years, 4 months ago (2012-08-10 23:33:40 UTC) #4
commit-bot: I haz the power
8 years, 4 months ago (2012-08-11 01:51:32 UTC) #5
Change committed as 151178

Powered by Google App Engine
This is Rietveld 408576698