Fix crash in SyncSetupWizard::IsVisible()

chrome/browser/ui/browser_navigator.cc

Index: chrome/browser/ui/browser_navigator.cc
diff --git a/chrome/browser/ui/browser_navigator.cc b/chrome/browser/ui/browser_navigator.cc
index ad0a54163d3cd15e40b1e1e328da00627fd02dd4..fe244cb8005a588f857c300a211a9faaf4139c0b 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -71,36 +71,37 @@ bool CompareURLsWithReplacements(
 }
 
 // Change some of the navigation parameters based on the particular URL.
-// Currently this applies to chrome://settings, the bookmark manager,
-// and chrome://extensions, which we always want to open in a normal
-// (not incognito) window. Guest session is an exception.
+// Currently this applies to some chrome:// pages which we always want to open
+// in a normal (not incognito) window. Guest session is an exception.

[James Hawkins, 2011/11/17 02:23:53]

s/normal (non incognito)/non-incognito/

[James Hawkins, 2011/11/17 02:23:53]

While you're here, can you clarify the guest session comment?

[binji, 2011/11/17 21:02:09]

Done.

[binji, 2011/11/17 21:02:09]

Done.

 // chrome://extensions is on the list because it redirects to
 // chrome://settings.
 // Returns true on success. Otherwise, if changing params leads the browser into
 // an erroneous state, returns false.
 bool AdjustNavigateParamsForURL(browser::NavigateParams* params) {
-  if (!params->target_contents &&
-      browser::IsURLAllowedInIncognito(params->url)) {
-    Profile* profile =
-        params->browser ? params->browser->profile() : params->profile;
-
-    if ((profile->IsOffTheRecord() && !Profile::IsGuestSession()) ||
-        params->disposition == OFF_THE_RECORD) {
-      profile = profile->GetOriginalProfile();
-
-      // If incognito is forced, we punt.
-      PrefService* prefs = profile->GetPrefs();
-      if (prefs && IncognitoModePrefs::GetAvailability(prefs) ==
-              IncognitoModePrefs::FORCED) {
-        return false;
-      }
+  if (params->target_contents != NULL ||

[binji, 2011/11/17 01:23:51]

I think it is clearer to early out here.

[James Hawkins, 2011/11/17 02:23:53]

Always and forever.

+      browser::IsURLAllowedInIncognito(params->url))
+    return true;
+
+  Profile* profile =
+      params->browser ? params->browser->profile() : params->profile;
 
-      params->disposition = SINGLETON_TAB;
-      params->profile = profile;
-      params->browser = Browser::GetOrCreateTabbedBrowser(profile);
-      params->window_action = browser::NavigateParams::SHOW_WINDOW;
+  if ((profile->IsOffTheRecord() && !Profile::IsGuestSession()) ||
+      params->disposition == OFF_THE_RECORD) {
+    profile = profile->GetOriginalProfile();
+
+    // If incognito is forced, we punt.
+    PrefService* prefs = profile->GetPrefs();
+    if (prefs && IncognitoModePrefs::GetAvailability(prefs) ==
+            IncognitoModePrefs::FORCED) {
+      return false;
     }
+
+    params->disposition = SINGLETON_TAB;
+    params->profile = profile;
+    params->browser = Browser::GetOrCreateTabbedBrowser(profile);
+    params->window_action = browser::NavigateParams::SHOW_WINDOW;
   }
+
   return true;
 }
 
@@ -632,10 +633,12 @@ int GetIndexOfSingletonTab(browser::NavigateParams* params) {
 }
 
 bool IsURLAllowedInIncognito(const GURL& url) {
-  return url.scheme() == chrome::kChromeUIScheme &&
+  // Most URLs are allowed in incognito; the following are exceptions.
+  return !(url.scheme() == chrome::kChromeUIScheme &&

[binji, 2011/11/17 01:23:51]

Reversed the logic here because the name was inconsistent with the logic.

[James Hawkins, 2011/11/17 02:23:53]

Don't you need to reverse the call sites too?

[binji, 2011/11/17 21:02:09]

Good catch.

       (url.host() == chrome::kChromeUISettingsHost ||
        url.host() == chrome::kChromeUIExtensionsHost ||
-       url.host() == chrome::kChromeUIBookmarksHost);
+       url.host() == chrome::kChromeUIBookmarksHost ||
+       url.host() == chrome::kChromeUISyncPromoHost));
 }
 
 }  // namespace browser