Using WebContents::UpdateTitleForEntry() instead of NavigationEntry::SetTitle()

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 23a03d9b75fac5e2fd7bb91fa7001bfe8296fbcc..5d923dfbe91b041b90187d396179ce6f494444ba 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1086,6 +1086,47 @@ const base::string16& WebContentsImpl::GetTitle() const {
   return page_title_when_no_navigation_entry_;
 }
 
+void WebContentsImpl::UpdateTitleForEntry(NavigationEntry* entry,

[Charlie Reis, 2016/07/13 21:02:23]

nit: I think it's probably better to leave this in the old location.  That would
preserve the blame history, and it would make reviewing the changes to it easier
as well.

[afakhry, 2016/07/13 22:10:27]

Done.

+                                          const base::string16& title) {
+  // For file URLs without a title, use the pathname instead. In the case of a
+  // synthesized title, we don't want the update to count toward the "one set
+  // per page of the title to history."
+  base::string16 final_title;
+  bool explicit_set;
+  if (entry && entry->GetURL().SchemeIsFile() && title.empty()) {
+    final_title = base::UTF8ToUTF16(entry->GetURL().ExtractFileName());
+    explicit_set = false;  // Don't count synthetic titles toward the set limit.
+  } else {
+    base::TrimWhitespace(title, base::TRIM_ALL, &final_title);
+    explicit_set = true;
+  }
+
+  // If a page is created via window.open and never navigated,
+  // there will be no navigation entry. In this situation,
+  // |page_title_when_no_navigation_entry_| will be used for page title.
+  if (entry) {
+    if (final_title == entry->GetTitle())
+      return;  // Nothing changed, don't bother.
+
+    entry->SetTitle(final_title);
+  } else {
+    if (page_title_when_no_navigation_entry_ == final_title)
+      return;  // Nothing changed, don't bother.
+
+    page_title_when_no_navigation_entry_ = final_title;
+  }
+
+  // Lastly, set the title for the view.
+  view_->SetPageTitle(final_title);
+
+  FOR_EACH_OBSERVER(WebContentsObserver, observers_,
+                    TitleWasSet(entry, explicit_set));
+
+  // Broadcast notifications when the UI should be updated.
+  if (entry == controller_.GetEntryAtOffset(0))
+    NotifyNavigationStateChanged(INVALIDATE_TYPE_TITLE);
+}
+
 int32_t WebContentsImpl::GetMaxPageID() {
   return GetMaxPageIDForSiteInstance(GetSiteInstance());
 }
@@ -3880,45 +3921,6 @@ void WebContentsImpl::UpdateMaxPageIDIfNecessary(RenderViewHost* rvh) {
                                    max_restored_page_id);
 }
 
-bool WebContentsImpl::UpdateTitleForEntry(NavigationEntryImpl* entry,
-                                          const base::string16& title) {
-  // For file URLs without a title, use the pathname instead. In the case of a
-  // synthesized title, we don't want the update to count toward the "one set
-  // per page of the title to history."
-  base::string16 final_title;
-  bool explicit_set;
-  if (entry && entry->GetURL().SchemeIsFile() && title.empty()) {
-    final_title = base::UTF8ToUTF16(entry->GetURL().ExtractFileName());
-    explicit_set = false;  // Don't count synthetic titles toward the set limit.
-  } else {
-    base::TrimWhitespace(title, base::TRIM_ALL, &final_title);
-    explicit_set = true;
-  }
-
-  // If a page is created via window.open and never navigated,
-  // there will be no navigation entry. In this situation,
-  // |page_title_when_no_navigation_entry_| will be used for page title.
-  if (entry) {
-    if (final_title == entry->GetTitle())
-      return false;  // Nothing changed, don't bother.
-
-    entry->SetTitle(final_title);
-  } else {
-    if (page_title_when_no_navigation_entry_ == final_title)
-      return false;  // Nothing changed, don't bother.
-
-    page_title_when_no_navigation_entry_ = final_title;
-  }
-
-  // Lastly, set the title for the view.
-  view_->SetPageTitle(final_title);
-
-  FOR_EACH_OBSERVER(WebContentsObserver, observers_,
-                    TitleWasSet(entry, explicit_set));
-
-  return true;
-}
-
 void WebContentsImpl::SendChangeLoadProgress() {
   loading_last_progress_update_ = base::TimeTicks::Now();
   if (delegate_)
@@ -4522,12 +4524,7 @@ void WebContentsImpl::UpdateTitle(RenderFrameHost* render_frame_host,
 
   // TODO(evan): make use of title_direction.
   // http://code.google.com/p/chromium/issues/detail?id=27094
-  if (!UpdateTitleForEntry(entry, title))
-    return;
-
-  // Broadcast notifications when the UI should be updated.
-  if (entry == controller_.GetEntryAtOffset(0))
-    NotifyNavigationStateChanged(INVALIDATE_TYPE_TITLE);
+  UpdateTitleForEntry(entry, title);
 }
 
 void WebContentsImpl::UpdateEncoding(RenderFrameHost* render_frame_host,