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

content/browser/web_contents/web_contents_impl.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 "content/browser/web_contents/web_contents_impl.h"
 
 #include <stddef.h>
 
 #include <cmath>
 #include <utility>
@@ skipping 1068 matching lines @@
 
   if (entry) {
     return entry->GetTitleForDisplay();
   }
 
   // |page_title_when_no_navigation_entry_| is finally used
   // if no title cannot be retrieved.
   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());
 }
 
 int32_t WebContentsImpl::GetMaxPageIDForSiteInstance(
     SiteInstance* site_instance) {
   if (max_page_ids_.find(site_instance->GetId()) == max_page_ids_.end())
     max_page_ids_[site_instance->GetId()] = -1;
 
   return max_page_ids_[site_instance->GetId()];
@@ skipping 2774 matching lines @@
   // navigating (to avoid a race between the browser updating max_page_id and
   // the renderer updating next_page_id_).  Because of this, we only call this
   // from CreateRenderView and allow that to notify the RenderView for us.
   int max_restored_page_id = controller_.GetMaxRestoredPageID();
   if (max_restored_page_id >
       GetMaxPageIDForSiteInstance(rvh->GetSiteInstance()))
     UpdateMaxPageIDForSiteInstance(rvh->GetSiteInstance(),
                                    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_)
     delegate_->LoadProgressChanged(this, frame_tree_.load_progress());
 }
 
 void WebContentsImpl::ResetLoadProgressState() {
   frame_tree_.ResetLoadProgress();
   loading_weak_factory_.InvalidateWeakPtrs();
   loading_last_progress_update_ = base::TimeTicks();
@@ skipping 583 matching lines @@
   }
 
   // We can handle title updates when we don't have an entry in
   // UpdateTitleForEntry, but only if the update is from the current RVH.
   // TODO(avi): Change to make decisions based on the RenderFrameHost.
   if (!entry && render_frame_host != GetMainFrame())
     return;
 
   // 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,
                                      const std::string& encoding) {
   SetEncoding(encoding);
 }
 
 void WebContentsImpl::DocumentAvailableInMainFrame(
     RenderViewHost* render_view_host) {
   FOR_EACH_OBSERVER(WebContentsObserver, observers_,
@@ skipping 620 matching lines @@
   for (RenderViewHost* render_view_host : render_view_host_set)
     render_view_host->OnWebkitPreferencesChanged();
 }
 
 void WebContentsImpl::SetJavaScriptDialogManagerForTesting(
     JavaScriptDialogManager* dialog_manager) {
   dialog_manager_ = dialog_manager;
 }
 
 }  // namespace content