Fix sync promo first run bugs

chrome/browser/ui/startup/startup_browser_creator_impl.cc

 // Copyright (c) 2012 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/startup/startup_browser_creator_impl.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "apps/app_restore_service.h"
@@ skipping 233 matching lines @@
     }
   }
   for (size_t i = 0; i < autolaunch_tabs.size(); ++i) {
     if (extension_service->IsInstalledApp(autolaunch_tabs.at(i).url)) {
       AppLauncherHandler::RecordAppLaunchType(
           extension_misc::APP_LAUNCH_AUTOLAUNCH);
     }
   }
 }
 
+bool IsNewTabURL(const GURL& url, Profile* profile) {

[sky, 2013/03/08 16:04:19]

IsNewTabOrHomePage()

[sail, 2013/03/08 20:06:43]

The "OrHomePage()" part isn't true. It's only if the url is a home page url
(empty) AND the home page is set to the NTP.

+  GURL ntp_url(chrome::kChromeUINewTabURL);
+  return url == ntp_url ||
+         (url.is_empty() && profile->GetHomePage() == ntp_url);
+}
+
+void AddSyncPromoTab(StartupTabs* tabs, Profile* profile) {

[sky, 2013/03/08 16:04:19]

out params should be last, so profile first then tabs.

[sail, 2013/03/08 20:06:43]

Done.

+  SyncPromoUI::DidShowSyncPromoAtStartup(profile);
+
+  StartupTab sync_promo_tab;
+  GURL continue_url;
+  if (!SyncPromoUI::UseWebBasedSigninFlow())
+    continue_url = GURL(chrome::kChromeUINewTabURL);
+  sync_promo_tab.url = SyncPromoUI::GetSyncPromoURL(
+      continue_url, SyncPromoUI::SOURCE_START_PAGE, false);
+  sync_promo_tab.is_pinned = false;
+  tabs->insert(tabs->begin(), sync_promo_tab);
+
+  // If the next URL is the NTP then remove it, effectively replacing the NTP
+  // with the sync promo.
+  if (tabs->size() > 1 && IsNewTabURL(tabs->at(1).url, profile))
+    tabs->erase(tabs->begin() + 1);

[gab, 2013/03/08 14:39:32]

This will remove the NTP even if it was intentionally added as the 1st
first_run_tabs; is that intended?

[sail, 2013/03/08 20:06:43]

This behavior is intentional. When the sync promo is completed (or skipped) we
redirect to the NTP. Replacing the NTP page means we won't have a duplicate NTP
tab.

+}
+
 class WebContentsCloseObserver : public content::NotificationObserver {
  public:
   WebContentsCloseObserver() : contents_(NULL) {}
   virtual ~WebContentsCloseObserver() {}
 
   void SetContents(content::WebContents* contents) {
     DCHECK(!contents_);
     contents_ = contents;
 
     registrar_.Add(this,
@@ skipping 435 matching lines @@
     // Only use the set of urls specified in preferences if nothing was
     // specified on the command line. Filter out any urls that are to be
     // restored by virtue of having been previously pinned.
     AddUniqueURLs(pref.urls, &tabs);
   } else if (pref.type == SessionStartupPref::HOMEPAGE) {
     // If 'homepage' selected, either by the user or by a policy, we should
     // have migrated them to another value.
     NOTREACHED() << "SessionStartupPref has deprecated type HOMEPAGE";
   }
 
+  if (pref.type != SessionStartupPref::LAST &&
+      SyncPromoUI::ShouldShowSyncPromoAtStartup(profile_, is_first_run_)) {
+    AddSyncPromoTab(&tabs, profile_);
+  }
+
   if (tabs.empty())
     return NULL;
 
   Browser* browser = OpenTabsInBrowser(NULL, true, tabs);
   return browser;
 }
 
 void StartupBrowserCreatorImpl::AddUniqueURLs(const std::vector<GURL>& urls,
                                               StartupTabs* tabs) {
   size_t num_existing_tabs = tabs->size();
@@ skipping 183 matching lines @@
       command_line_.HasSwitch(switches::kResetLocalPassphrase);
   if ((is_first_run_ || has_reset_local_passphrase_switch) &&
       prefs->GetBoolean(prefs::kProfileIsManaged)) {
     startup_urls->insert(startup_urls->begin(),
                          GURL(std::string(chrome::kChromeUISettingsURL) +
                               chrome::kManagedUserSettingsSubPage));
     if (has_reset_local_passphrase_switch) {
       prefs->SetString(prefs::kManagedModeLocalPassphrase, "");
       prefs->SetString(prefs::kManagedModeLocalSalt, "");
     }
-  } else if (SyncPromoUI::ShouldShowSyncPromoAtStartup(profile_,
-                                                       is_first_run_)) {
-    // If the sync promo page is going to be displayed then insert it at the
-    // front of the list.
-    GURL continue_url;
-    if (!SyncPromoUI::UseWebBasedSigninFlow()) {
-      continue_url = GURL(chrome::kChromeUINewTabURL);
-    }
-
-    SyncPromoUI::DidShowSyncPromoAtStartup(profile_);
-    GURL old_url = (*startup_urls)[0];
-    (*startup_urls)[0] =
-        SyncPromoUI::GetSyncPromoURL(continue_url,
-                                     SyncPromoUI::SOURCE_START_PAGE,
-                                     false);
-
-    // An empty URL means to go to the home page.
-    if (old_url.is_empty() &&
-        profile_->GetHomePage() == GURL(chrome::kChromeUINewTabURL)) {
-      old_url = GURL(chrome::kChromeUINewTabURL);
-    }
-
-    // If the old URL is not the NTP then insert it right after the sync promo.
-    if (old_url != GURL(chrome::kChromeUINewTabURL))
-      startup_urls->insert(startup_urls->begin() + 1, old_url);
   }
 }
 
 #if !defined(OS_WIN) || defined(USE_AURA)
 // static
 bool StartupBrowserCreatorImpl::OpenStartupURLsInExistingBrowser(
     Profile* profile,
     const std::vector<GURL>& startup_urls) {
   return false;
 }
 #endif