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

Issue 7906013: Show bubble and wrench menu item for sync error (Closed)

Created:
9 years, 3 months ago by sail
Modified:
9 years, 3 months ago
CC:
chromium-reviews, Raghu Simha, ncarter (slow), tim (not reviewing), idana
Visibility:
Public.

Description

Show bubble and wrench menu item for sync error With this change we now show a bubble and menu item for actionable sync errors. BUG=92803 TEST= Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102061

Patch Set 1 #

Total comments: 4

Patch Set 2 : Address review comments #

Total comments: 4

Patch Set 3 : Refactor into sync_ui_util #

Total comments: 2

Patch Set 4 : Address review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+347 lines, -34 lines) Patch
M chrome/browser/sync/profile_sync_service.h View 1 2 3 3 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/sync/profile_sync_service.cc View 1 2 3 3 chunks +17 lines, -0 lines 0 comments Download
A chrome/browser/sync/sync_global_error.h View 1 2 1 chunk +53 lines, -0 lines 0 comments Download
A chrome/browser/sync/sync_global_error.cc View 1 2 1 chunk +102 lines, -0 lines 0 comments Download
M chrome/browser/sync/sync_ui_util.h View 1 2 3 2 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/sync/sync_ui_util.cc View 1 2 3 4 chunks +132 lines, -34 lines 0 comments Download
M chrome/browser/ui/toolbar/wrench_menu_model.cc View 1 2 3 4 chunks +28 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
sail
sky: ui/toolbar stuff jhawkins: sync error stuff akalin: OWNERs approval (not sure who to add ...
9 years, 3 months ago (2011-09-15 19:34:04 UTC) #1
James Hawkins
Recusing myself since I'm not an OWNER of sync/. http://codereview.chromium.org/7906013/diff/1/chrome/browser/sync/sync_global_error.h File chrome/browser/sync/sync_global_error.h (right): http://codereview.chromium.org/7906013/diff/1/chrome/browser/sync/sync_global_error.h#newcode16 chrome/browser/sync/sync_global_error.h:16: ...
9 years, 3 months ago (2011-09-15 19:40:25 UTC) #2
sky
LGTM http://codereview.chromium.org/7906013/diff/1/chrome/browser/sync/sync_global_error.cc File chrome/browser/sync/sync_global_error.cc (right): http://codereview.chromium.org/7906013/diff/1/chrome/browser/sync/sync_global_error.cc#newcode131 chrome/browser/sync/sync_global_error.cc:131: nit: remove this line.
9 years, 3 months ago (2011-09-15 19:52:07 UTC) #3
akalin
I think Tim would be a better sync reviewer for this
9 years, 3 months ago (2011-09-15 20:05:13 UTC) #4
sail
http://codereview.chromium.org/7906013/diff/1/chrome/browser/sync/sync_global_error.cc File chrome/browser/sync/sync_global_error.cc (right): http://codereview.chromium.org/7906013/diff/1/chrome/browser/sync/sync_global_error.cc#newcode131 chrome/browser/sync/sync_global_error.cc:131: On 2011/09/15 19:52:07, sky wrote: > nit: remove this ...
9 years, 3 months ago (2011-09-15 20:22:51 UTC) #5
tim (not reviewing)
http://codereview.chromium.org/7906013/diff/5001/chrome/browser/sync/sync_global_error.cc File chrome/browser/sync/sync_global_error.cc (right): http://codereview.chromium.org/7906013/diff/5001/chrome/browser/sync/sync_global_error.cc#newcode138 chrome/browser/sync/sync_global_error.cc:138: switch (auth_error.state()) { We have a pile of code ...
9 years, 3 months ago (2011-09-16 22:14:24 UTC) #6
sail
http://codereview.chromium.org/7906013/diff/5001/chrome/browser/sync/sync_global_error.cc File chrome/browser/sync/sync_global_error.cc (right): http://codereview.chromium.org/7906013/diff/5001/chrome/browser/sync/sync_global_error.cc#newcode138 chrome/browser/sync/sync_global_error.cc:138: switch (auth_error.state()) { On 2011/09/16 22:14:24, timsteele wrote: > ...
9 years, 3 months ago (2011-09-16 22:25:07 UTC) #7
sail
http://codereview.chromium.org/7906013/diff/5001/chrome/browser/sync/sync_global_error.cc File chrome/browser/sync/sync_global_error.cc (right): http://codereview.chromium.org/7906013/diff/5001/chrome/browser/sync/sync_global_error.cc#newcode138 chrome/browser/sync/sync_global_error.cc:138: switch (auth_error.state()) { On 2011/09/16 22:25:07, sail wrote: > ...
9 years, 3 months ago (2011-09-16 22:26:16 UTC) #8
sail
Refactored error testing code into sync_ui_util. http://codereview.chromium.org/7906013/diff/5001/chrome/browser/sync/sync_global_error.cc File chrome/browser/sync/sync_global_error.cc (right): http://codereview.chromium.org/7906013/diff/5001/chrome/browser/sync/sync_global_error.cc#newcode138 chrome/browser/sync/sync_global_error.cc:138: switch (auth_error.state()) { ...
9 years, 3 months ago (2011-09-17 01:15:30 UTC) #9
tim (not reviewing)
lgtm http://codereview.chromium.org/7906013/diff/10001/chrome/browser/sync/sync_ui_util.cc File chrome/browser/sync/sync_ui_util.cc (right): http://codereview.chromium.org/7906013/diff/10001/chrome/browser/sync/sync_ui_util.cc#newcode35 chrome/browser/sync/sync_ui_util.cc:35: // Given an authentication state this hellper function ...
9 years, 3 months ago (2011-09-20 03:26:47 UTC) #10
sail
http://codereview.chromium.org/7906013/diff/10001/chrome/browser/sync/sync_ui_util.cc File chrome/browser/sync/sync_ui_util.cc (right): http://codereview.chromium.org/7906013/diff/10001/chrome/browser/sync/sync_ui_util.cc#newcode35 chrome/browser/sync/sync_ui_util.cc:35: // Given an authentication state this hellper function returns ...
9 years, 3 months ago (2011-09-20 21:45:43 UTC) #11
commit-bot: I haz the power
9 years, 3 months ago (2011-09-21 00:52:54 UTC) #12
Change committed as 102061

Powered by Google App Engine
This is Rietveld 408576698