Use the original profile to get the sync service when building the app menu....

Use the original profile to get the sync service when building the app menu.

BUG=23993
TEST=Open incognito window. Click the bookmark sync button. Either the signin dialog
or the options panel should open.


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

[ncarter (slow), 2009-10-30 18:25:27]

Nice fix.

On Fri, Oct 30, 2009 at 11:15 AM, <tim@chromium.org> wrote:

>
> Reviewers: idana,
>
> Description:
> Use the original profile to get the sync service when building the app
> menu.
>
> BUG=23993
> TEST=Open incognito window. Click the bookmark sync button. Either the
> signin
> dialog
> or the options panel should open.
>
>
> Please review this at http://codereview.chromium.org/350005
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
>  M     chrome/browser/browser.cc
>  M     chrome/browser/views/toolbar_view.cc
>
>
> Index: chrome/browser/views/toolbar_view.cc
> ===================================================================
> --- chrome/browser/views/toolbar_view.cc        (revision 30303)
> +++ chrome/browser/views/toolbar_view.cc        (working copy)
> @@ -1179,7 +1179,8 @@
>     // TODO(timsteele): Need a ui helper method to just get the type
> without
>     // needing labels.
>     SyncStatusUIHelper::MessageType type = SyncStatusUIHelper::GetLabels(
> -        browser_->profile()->GetProfileSyncService(), &label, &link);
> +
>  browser_->profile()->GetOriginalProfile()->GetProfileSyncService(),
> +        &label, &link);
>     label = type == SyncStatusUIHelper::SYNCED ?
>         l10n_util::GetStringUTF16(IDS_SYNC_MENU_BOOKMARKS_SYNCED_LABEL) :
>         type == SyncStatusUIHelper::SYNC_ERROR ?
> Index: chrome/browser/browser.cc
> ===================================================================
> --- chrome/browser/browser.cc   (revision 30303)
> +++ chrome/browser/browser.cc   (working copy)
> @@ -654,8 +654,8 @@
>   command_updater_.UpdateCommandEnabled(IDC_SHOW_BOOKMARK_BAR,
> show_main_ui);
>   command_updater_.UpdateCommandEnabled(IDC_IMPORT_SETTINGS, show_main_ui);
>   command_updater_.UpdateCommandEnabled(
> -      IDC_SYNC_BOOKMARKS,
> -      show_main_ui && profile_->GetProfileSyncService() != NULL);
> +      IDC_SYNC_BOOKMARKS, show_main_ui &&
> +          profile_->GetOriginalProfile()->GetProfileSyncService() !=
> NULL);
>   command_updater_.UpdateCommandEnabled(IDC_OPTIONS, show_main_ui);
>   command_updater_.UpdateCommandEnabled(IDC_EDIT_SEARCH_ENGINES,
> show_main_ui);
>   command_updater_.UpdateCommandEnabled(IDC_VIEW_PASSWORDS, show_main_ui);
> @@ -1201,9 +1201,9 @@
>  }
>
>  void Browser::OpenSyncMyBookmarksDialog() {
> -  ProfileSyncService* service = profile_->GetProfileSyncService();
> -  // TODO(timsteele): Incognito has no sync service for the time being,
> -  // so protect against this case.
> +  ProfileSyncService* service =
> +      profile_->GetOriginalProfile()->GetProfileSyncService();
> +  // It is possible to have no sync service, just do nothing in this case.
>   if (!service)
>     return;
>   if (service->HasSyncSetupCompleted()) {
>
>
>

[idana, 2009-10-30 18:42:48]

LGTM

http://codereview.chromium.org/350005/diff/1003/5
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/350005/diff/1003/5#newcode1208
Line 1208: return;
Since it shouldn't be possible for this function to get called if sync is
disabled, you should probably add a DCHECK().