Don't rely on |entry| being null in initial title logic.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 1ee7a7dc3807794f0707f36abbef980e1c51da21..9b53030c2e68f7da57b9334373d903d50b6ded33 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -946,21 +946,19 @@ const base::string16& WebContentsImpl::GetTitle() const {
   // title.
   entry = controller_.GetLastCommittedEntry();
 
-  // We make an exception for initial navigations.
-  if (controller_.IsInitialNavigation()) {
-    // We only want to use the title from the visible entry in one of two cases:
-    // 1. There's already a committed entry for an initial navigation, in which
-    //    case we are doing a history navigation in a new tab (e.g., Ctrl+Back).
-    // 2. The pending entry has been explicitly assigned a title to display.
-    //
-    // If there's no last committed entry and no assigned title, we should fall
-    // back to |page_title_when_no_navigation_entry_| rather than showing the
-    // URL.
-    if (entry ||

[Avi (use Gerrit), 2015/09/28 19:11:39]

I'm not sure what this was supposed to mean. How could there be a last-committed
entry if the controller is in the initial navigation? A commit would have gone
through NavigationControllerImpl::RendererDidNavigate, which would have cleared
the initial navigation flag.

[Charlie Reis, 2015/09/28 19:21:41]

I thought the same at first glance, but there's actually a case with a "last
committed entry" during the initial navigation: Ctrl+Back.  This clones the tab
(and somewhat confusingly, the last committed entry index), but the initial
navigation is the Back navigation.

One of many fun cases I encountered in the epic https://crbug.com/9682.

-        (controller_.GetVisibleEntry() &&
-         !controller_.GetVisibleEntry()->GetTitle().empty())) {
-      entry = controller_.GetVisibleEntry();
-    }
+  // We make an exception for initial navigations. We only want to use the title
+  // from the visible entry if:
+  // 1. The pending entry has been explicitly assigned a title to display.
+  // 2. The user is doing a history navigation in a new tab (e.g., Ctrl+Back),
+  //    which case there is a pending entry index other than -1.
+  //
+  // Otherwise, we want to stick with the last committed entry's title during
+  // new navigations, which have pending entries at index -1 with no title.
+  if (controller_.IsInitialNavigation() &&
+      ((controller_.GetVisibleEntry() &&

[Charlie Reis, 2015/09/28 18:18:43]

Note: I'll be able to remove this line once we always have a last committed
entry, making this version a bit simpler and easier to read.

[Avi (use Gerrit), 2015/09/28 19:11:39]

Will you then (please) also kill
WebContentsImpl::page_title_when_no_navigation_entry_ ?

[Charlie Reis, 2015/09/28 19:21:41]

Yep!  There's a lot of cruft like that on the chopping block.  :)

+        !controller_.GetVisibleEntry()->GetTitle().empty()) ||
+       controller_.GetPendingEntryIndex() != -1)) {
+    entry = controller_.GetVisibleEntry();
   }
 
   if (entry) {