Don't update profile information when the sync state changes.

Don't update profile information when the sync state changes.

BUG=262104
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215883

[Bernhard Bauer, 2013-08-05 17:17:09]

Please review.

[Adrian Kuegel, 2013-08-06 10:05:35]

https://codereview.chromium.org/22216002/diff/1/chrome/browser/ui/webui/optio...
File chrome/browser/ui/webui/options/browser_options_handler.cc (left):

https://codereview.chromium.org/22216002/diff/1/chrome/browser/ui/webui/optio...
chrome/browser/ui/webui/options/browser_options_handler.cc:664:
SendProfilesInfo();
I haven't completely understood if removing this line might break something, but
from what I can tell the call should be covered by the
chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED.
Anyway, the documentation of the OnStateChanged method seems to suggest to query
the sync service about what kind of change occurred, so that might also be an
option.

https://codereview.chromium.org/22216002/diff/1/chrome/browser/ui/webui/optio...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/22216002/diff/1/chrome/browser/ui/webui/optio...
chrome/browser/ui/webui/options/browser_options_handler.cc:664: void
BrowserOptionsHandler::UpdateSyncState() {
Can you move this function (and OnSigninAllowedPrefChange) down corresponding to
their position in the header file?
I guess the functions in this file should at least be ordered by public
functions first, then private.

[Bernhard Bauer, 2013-08-06 10:38:49]

https://codereview.chromium.org/22216002/diff/1/chrome/browser/ui/webui/optio...
File chrome/browser/ui/webui/options/browser_options_handler.cc (left):

https://codereview.chromium.org/22216002/diff/1/chrome/browser/ui/webui/optio...
chrome/browser/ui/webui/options/browser_options_handler.cc:664:
SendProfilesInfo();
On 2013/08/06 10:05:35, Adrian Kuegel wrote:
> I haven't completely understood if removing this line might break something,
but
> from what I can tell the call should be covered by the
> chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED.

Yes, I traced this back to when it was added originally, and the two updates
were just lumped together in the same method (OnStateChanged sounds kind of
general).

> Anyway, the documentation of the OnStateChanged method seems to suggest to
query
> the sync service about what kind of change occurred, so that might also be an
> option.

Well, ProfileSyncService sends the notification among other things after every
sync cycle, so nothing would even have changed there.

https://codereview.chromium.org/22216002/diff/1/chrome/browser/ui/webui/optio...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/22216002/diff/1/chrome/browser/ui/webui/optio...
chrome/browser/ui/webui/options/browser_options_handler.cc:664: void
BrowserOptionsHandler::UpdateSyncState() {
On 2013/08/06 10:05:35, Adrian Kuegel wrote:
> Can you move this function (and OnSigninAllowedPrefChange) down corresponding
to
> their position in the header file?
> I guess the functions in this file should at least be ordered by public
> functions first, then private.

Done.

[Adrian Kuegel, 2013-08-06 10:40:27]

lgtm

[commit-bot: I haz the power, 2013-08-06 10:40:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/22216002/13001

[commit-bot: I haz the power, 2013-08-06 13:30:57]

Change committed as 215883