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

chrome/browser/search/search.cc

Index: chrome/browser/search/search.cc
diff --git a/chrome/browser/search/search.cc b/chrome/browser/search/search.cc
index 645fecb9a52e535b344cc9554331c891695ddc61..5a743c9bd1aad61bafd4b5fd10a5335b63000fda 100644
--- a/chrome/browser/search/search.cc
+++ b/chrome/browser/search/search.cc
@@ -42,6 +42,10 @@
 #include "chrome/browser/supervised_user/supervised_user_url_filter.h"
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/login/signin/merge_session_throttling_utils.h"
+#endif  // defined(OS_CHROMEOS)
+
 namespace search {
 
 namespace {
@@ -239,6 +243,22 @@ NewTabURLState IsValidNewTabURL(Profile* profile, const GURL& new_tab_url) {
   return NEW_TAB_URL_VALID;
 }
 
+// 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 ShouldShowLocalNewTab(const GURL& url, Profile* profile) {
+#if defined(OS_CHROMEOS)
+  if (!merge_session_throttling_utils::ShouldDelayUrl(url) ||
+      !merge_session_throttling_utils::ShouldDelayRequest(profile)) {

[Marc Treib, 2016/04/21 19:53:38]

Can you please swap the if around, to
if (ShouldDelayUrl(...) && ShouldDelayRequest(...)) return true;

Less negation makes for easier reading, and that way the "default case" for CrOS
and other platforms matches up.

Now that I look at it like this: Is it correct that you only want to show the
local NTP if *both* ShouldDelay functions return true? If so, would it make
sense to combine them into one ShouldDelay(profile, url) function?

[afakhry, 2016/04/22 00:41:14]

Done.

Regarding combining them into one function, I see both are used separately in a
couple of places. So I don't think we should combine them.

+    return false;
+  }
+
+  return true;
+#else
+  return false;
+#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 {
@@ -259,6 +279,10 @@ struct NewTabURLDetails {
     GURL search_provider_url = TemplateURLRefToGURL(
         template_url->new_tab_url_ref(), UIThreadSearchTermsData(profile),
         false, false);
+
+    if (ShouldShowLocalNewTab(search_provider_url, profile))
+      return NewTabURLDetails(local_url, NEW_TAB_URL_VALID);
+
     NewTabURLState state = IsValidNewTabURL(profile, search_provider_url);
     switch (state) {
       case NEW_TAB_URL_VALID: