DOMUI: Move the meat of SyncOptionsHandler into PersonalOptionsHandler.

DOMUI: Move the meat of SyncOptionsHandler into PersonalOptionsHandler.

(This was a TODO for jhawkins).

BUG=None
TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71615

[tfarina, 2011-01-16 04:12:35]

Hi James, could you review this to me?

[James Hawkins, 2011-01-17 22:34:17]

Please thoroughly review changes you make in uploaded CLs. There is a glaring
error in this CL, and it appears you merely copied code from SyncOptionsHandler
into PersonalOptionsHandler without analyzing the repercussions.

http://codereview.chromium.org/6344004/diff/1/chrome/browser/dom_ui/options/p...
File chrome/browser/dom_ui/options/personal_options_handler.cc (right):

http://codereview.chromium.org/6344004/diff/1/chrome/browser/dom_ui/options/p...
chrome/browser/dom_ui/options/personal_options_handler.cc:55: return
ProfileSyncService::IsSyncEnabled();
This is wrong.

[tfarina, 2011-01-17 23:34:08]

http://codereview.chromium.org/6344004/diff/1/chrome/browser/dom_ui/options/p...
File chrome/browser/dom_ui/options/personal_options_handler.cc (right):

http://codereview.chromium.org/6344004/diff/1/chrome/browser/dom_ui/options/p...
chrome/browser/dom_ui/options/personal_options_handler.cc:55: return
ProfileSyncService::IsSyncEnabled();
On 2011/01/17 22:34:17, James Hawkins wrote:
> This is wrong.

Done.

[James Hawkins, 2011-01-17 23:46:18]

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/personal_options_handler.cc (right):

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:130: return;
The sync strings should be loaded regardless of whether Sync happens to be
enabled when this page is initialized or not.

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:132: // Sync page -
ChromeOS
s/ - ChromeOS/./

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:170: if
(!ProfileSyncService::IsSyncEnabled())
Same as above.

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:309: if
(!sync_service)
if (sync_service) is 7 lines above. Please refactor this logic.

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:312: if
(!ProfileSyncService::IsSyncEnabled())
As above, whether sync is enabled or not has nothing to do with initialization. 
That handling is done in OnStateChanged.

[tfarina, 2011-01-17 23:52:17]

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/personal_options_handler.cc (right):

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:130: return;
On 2011/01/17 23:46:18, James Hawkins wrote:
> The sync strings should be loaded regardless of whether Sync happens to be
> enabled when this page is initialized or not.

Removed.

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:132: // Sync page -
ChromeOS
On 2011/01/17 23:46:18, James Hawkins wrote:
> s/ - ChromeOS/./

Done.

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:170: if
(!ProfileSyncService::IsSyncEnabled())
On 2011/01/17 23:46:18, James Hawkins wrote:
> Same as above.

Removed.

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:309: if
(!sync_service)
On 2011/01/17 23:46:18, James Hawkins wrote:
> if (sync_service) is 7 lines above. Please refactor this logic.

Done.

http://codereview.chromium.org/6344004/diff/5001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/personal_options_handler.cc:312: if
(!ProfileSyncService::IsSyncEnabled())
On 2011/01/17 23:46:18, James Hawkins wrote:
> As above, whether sync is enabled or not has nothing to do with
initialization. 
> That handling is done in OnStateChanged.

Removed.

[James Hawkins, 2011-01-17 23:54:18]

LGTM, thanks.