New Tab: fix tab title flickering.

chrome/browser/ui/browser_navigator.cc

Index: chrome/browser/ui/browser_navigator.cc
diff --git a/chrome/browser/ui/browser_navigator.cc b/chrome/browser/ui/browser_navigator.cc
index ed4626879e66d24f9ecf09775b73b4898ddcc1b7..ee9133604b7555dc54d36968f6bfb4a48800a443 100644
--- a/chrome/browser/ui/browser_navigator.cc
+++ b/chrome/browser/ui/browser_navigator.cc
@@ -19,6 +19,7 @@
 #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"
@@ -34,11 +35,14 @@
 #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"
@@ -268,6 +272,29 @@ void LoadURLInContents(WebContents* target_contents,
         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