Avoid the long merge session throttle interstitial page for NTP on Chrome OS

chrome/browser/search/search.cc

 // Copyright 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/search/search.h"
 
 #include <stddef.h>
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/login/signin/merge_session_throttling_utils.h"

[Marc Treib, 2016/04/21 08:32:12]

This should go into an #if defined(OS_CHROMEOS) block.

[afakhry, 2016/04/21 17:04:09]

Done.

 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
 #include "chrome/browser/ui/search/instant_search_prerenderer.h"
 #include "chrome/common/chrome_switches.h"
@@ skipping 206 matching lines @@
     return NEW_TAB_URL_INCOGNITO;
   if (!new_tab_url.is_valid())
     return NEW_TAB_URL_NOT_SET;
   if (!new_tab_url.SchemeIsCryptographic())
     return NEW_TAB_URL_INSECURE;
   if (!IsURLAllowedForSupervisedUser(new_tab_url, profile))
     return NEW_TAB_URL_BLOCKED;
   return NEW_TAB_URL_VALID;
 }
 
+#if defined(OS_CHROMEOS)
+
+// On Chrome OS, if the session hasn't merged yet, we need to avoid loading the
+// remote NTP because that will trigger showing the merge session throttle
+// interstitial page, which can show for 5+ seconds. crbug.com/591530.
+bool ShouldShowLocalNtpWhileMergeSessionInProgress(const GURL& url,
+                                                   Profile* profile) {
+  if (!merge_session_throttling_utils::ShouldDelayUrl(url) ||

[Marc Treib, 2016/04/21 08:32:12]

I'd move the "#if defined(OS_CHROMEOS)" here so that it covers this "if" only,
and rename the function to ShouldShowLocalNewTab. Then you won't need another
OS-specific block below.

[afakhry, 2016/04/21 17:04:09]

Done.

+      !merge_session_throttling_utils::ShouldDelayRequest(profile)) {
+    return false;
+  }
+
+  return true;
+}
+
+#endif  // defined(OS_CHROMEOS)
+
 // Used to look up the URL to use for the New Tab page. Also tracks how we
 // arrived at that URL so it can be logged with UMA.
 struct NewTabURLDetails {
   NewTabURLDetails(const GURL& url, NewTabURLState state)
       : url(url), state(state) {}
 
   static NewTabURLDetails ForProfile(Profile* profile) {
     const GURL local_url(chrome::kChromeSearchLocalNtpUrl);
 
     base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
     if (command_line->HasSwitch(switches::kForceLocalNtp))
       return NewTabURLDetails(local_url, NEW_TAB_URL_VALID);
 
     TemplateURL* template_url = GetDefaultSearchProviderTemplateURL(profile);
     if (!profile || !template_url)
       return NewTabURLDetails(local_url, NEW_TAB_URL_BAD);
 
     GURL search_provider_url = TemplateURLRefToGURL(
         template_url->new_tab_url_ref(), UIThreadSearchTermsData(profile),
         false, false);
+
+#if defined(OS_CHROMEOS)
+    if (ShouldShowLocalNtpWhileMergeSessionInProgress(search_provider_url,
+                                                      profile)) {
+      // Force load the local NTP if the merge session is not complete.
+      return NewTabURLDetails(local_url, NEW_TAB_URL_VALID);
+    }
+#endif  // defined(OS_CHROMEOS)
+
     NewTabURLState state = IsValidNewTabURL(profile, search_provider_url);
     switch (state) {
       case NEW_TAB_URL_VALID:
         // We can use the search provider's page.
         return NewTabURLDetails(search_provider_url, state);
       case NEW_TAB_URL_INCOGNITO:
         // Incognito has its own New Tab.
         return NewTabURLDetails(GURL(), state);
       default:
         // Use the local New Tab otherwise.
@@ skipping 312 matching lines @@
       kUseAltInstantURL, false, flags);
 }
 
 bool ShouldUseSearchPathForInstant() {
   FieldTrialFlags flags;
   return GetFieldTrialInfo(&flags) && GetBoolValueForFlagWithDefault(
       kUseSearchPathForInstant, false, flags);
 }
 
 }  // namespace search