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, add a transient entry and explicitly set the

[sky, 2013/11/21 23:16:31]

This seems like a total hack to me. Isn't there a better way?

[samarth, 2013/11/21 23:20:58]

Yeah.  I tried setting the title on the pending entry, but that doesn't work,
because WebContentsImpl looks at the last committed entry.

It does have an exception[1] for "IsInitialNavigation" which sounds like it
should cover this case, but IsInitialNavigation is set to false at that point. 
Is that a bug?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/we...

[Charlie Reis, 2013/11/21 23:23:57]

It's worth understanding why IsInitialNavigation is returning false, at least. 
It does seem plausible for that to be true in this case (not sure), and you
definitely shouldn't be using the transient entry logic here.

[samarth, 2013/11/21 23:44:06]

Ah ok, so the problem is that the check is IsInitialNavigation &&
last_committed_entry != NULL.  In the case of the very first load, there is no
committed entry and IsInitialNavigation is true.

So one possible fix that works would be to:
1) Change WebContentsImpl::GetTitle to use the title from the pending entry
whenever IsInitialNavigation is true.
2) Here, set the title on the pending entry.

Charlie: would that introduce any security concerns?

(Actually looking at this more carefully, I don't understand how the existing
condition could ever be true.  is_initial_navigation_ starts out true, is set to
false once any entry is committed and never changed after that.  So how would it
ever be true if there is a valid committed entry?)

[samarth, 2013/11/22 00:29:03]

I implemented this in PS2. Let me know what you think.

+  // title to "New Tab".
+  //
+  // 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(
+        entry->GetURL(),
+        Profile::FromBrowserContext(target_contents->GetBrowserContext()))) {
+    content::NavigationEntry* transient = controller->CreateNavigationEntry(
+        entry->GetURL(),
+        entry->GetReferrer(),
+        entry->GetTransitionType(),
+        false,
+        std::string(),
+        target_contents->GetBrowserContext());
+    transient->SetTitle(l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE));
+    controller->SetTransientEntry(transient);
+  }
 }
 
 // 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