Fix crash in SyncSetupWizard::IsVisible()

chrome/browser/ui/browser_navigator.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/browser_navigator.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/stringprintf.h"
@@ skipping 53 matching lines @@
     const url_canon::Replacements<char>& replacements) {
   if (url == other)
     return true;
 
   GURL url_replaced = url.ReplaceComponents(replacements);
   GURL other_replaced = other.ReplaceComponents(replacements);
   return url_replaced == other_replaced;
 }
 
 // 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 (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;
 
-    if ((profile->IsOffTheRecord() && !Profile::IsGuestSession()) ||
-        params->disposition == OFF_THE_RECORD) {
-      profile = profile->GetOriginalProfile();
+  Profile* profile =
+      params->browser ? params->browser->profile() : params->profile;
 
-      // If incognito is forced, we punt.
-      PrefService* prefs = profile->GetPrefs();
-      if (prefs && IncognitoModePrefs::GetAvailability(prefs) ==
-              IncognitoModePrefs::FORCED) {
-        return false;
-      }
+  if ((profile->IsOffTheRecord() && !Profile::IsGuestSession()) ||
+      params->disposition == OFF_THE_RECORD) {
+    profile = profile->GetOriginalProfile();
 
-      params->disposition = SINGLETON_TAB;
-      params->profile = profile;
-      params->browser = Browser::GetOrCreateTabbedBrowser(profile);
-      params->window_action = browser::NavigateParams::SHOW_WINDOW;
+    // 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;
 }
 
 // Returns a Browser that can host the navigation or tab addition specified in
 // |params|. This might just return the same Browser specified in |params|, or
 // some other if that Browser is deemed incompatible.
 Browser* GetBrowserForDisposition(browser::NavigateParams* params) {
   // If no source TabContents was specified, we use the selected one from the
   // target browser. This must happen first, before GetBrowserForDisposition()
   // has a chance to replace |params->browser| with another one.
@@ skipping 511 matching lines @@
                                     rewritten_url, replacements)) {
       params->target_contents = tab;
       return tab_index;
     }
   }
 
   return -1;
 }
 
 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