New Tab: fix tab title flickering.

chrome/browser/ui/browser_navigator.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/browser_navigator.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_about_handler.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/google/google_url_tracker.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search/search.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
 #include "chrome/browser/ui/browser_tab_contents.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/omnibox/location_bar.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/status_bubble.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_url_handler.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "extensions/common/extension.h"
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
 
 #if defined(USE_AURA)
 #include "ui/aura/window.h"
 #endif
 
 using content::GlobalRequestID;
 using content::NavigationController;
 using content::WebContents;
 
 class BrowserNavigatorWebContentsAdoption {
@@ skipping 209 matching lines @@
   }
 
   // Only allows the browser-initiated navigation to use POST.
   if (params->uses_post && !params->is_renderer_initiated) {
     load_url_params.load_type =
         NavigationController::LOAD_TYPE_BROWSER_INITIATED_HTTP_POST;
     load_url_params.browser_initiated_post_data =
         params->browser_initiated_post_data;
   }
   target_contents->GetController().LoadURLWithParams(load_url_params);
+
+  // For Instant Extended NTPs, explicitly set the title to "New Tab" on the
+  // pending entry.
+  //
+  // This ensures that the title is set even before we get a title from the
+  // page, preventing a potential flicker of the URL or "Loading...", and also
+  // ensures that (unless overridden by the page) the new tab title matches the
+  // browser UI locale.
+  content::NavigationController* controller = &target_contents->GetController();
+  content::NavigationEntry* entry = controller->GetVisibleEntry();
+  if (chrome::IsNTPURL(

[Charlie Reis, 2013/11/22 18:54:18]

Hmm, this call happens for almost every URL.  (Soon it will happen for some
subframe navigations as well.)  It seems unfortunate to put checks for specific
URLs here.

Is there any way to set the title somewhere in the NTP logic instead, right
after starting the navigation?

[samarth, 2013/11/22 20:03:19]

Agreed, this doesn't seem ideal. I don't know where else to put the check
though. The way the NTP works right now is that
1) The browser starts a regular navigation to chrome://newtab
2) We rewrite the URL when appropriate here:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

Then the rest of the navigation continues as normal. I don't think the
ContentBrowserClient code has visibility into the naventry being used for the
navigation (but I could be wrong).

---

Ok, how about if I move this into SearchTabHelper and add a listener there for
NavigateToPendingEntry?  That essentially the same, but at least the logic is
contained in the correct place.

[Charlie Reis, 2013/11/22 20:33:47]

That sounds perfect-- thanks.

[samarth, 2013/11/22 21:54:41]

Done.

+        entry->GetURL(),
+        Profile::FromBrowserContext(target_contents->GetBrowserContext()))) {
+    entry->SetTitle(l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE));
+  }
 }
 
 // This class makes sure the Browser object held in |params| is made visible
 // by the time it goes out of scope, provided |params| wants it to be shown.
 class ScopedBrowserShower {
  public:
   explicit ScopedBrowserShower(chrome::NavigateParams* params)
       : params_(params) {
   }
   ~ScopedBrowserShower() {
@@ skipping 425 matching lines @@
   bool reverse_on_redirect = false;
   content::BrowserURLHandler::GetInstance()->RewriteURLIfNecessary(
       &rewritten_url, browser_context, &reverse_on_redirect);
 
   // Some URLs are mapped to uber subpages. Do not allow them in incognito.
   return !(rewritten_url.scheme() == chrome::kChromeUIScheme &&
            rewritten_url.host() == chrome::kChromeUIUberHost);
 }
 
 }  // namespace chrome