Fix flickering in new tab title after r444582.

chrome/browser/ui/search/search_tab_helper.cc

 // Copyright 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/search/search_tab_helper.h"
 
 #include <memory>
 #include <set>
 
 #include "base/memory/ptr_util.h"
@@ skipping 27 matching lines @@
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/referrer.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 #include "net/base/net_errors.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/page_transition_types.h"
 #include "url/gurl.h"
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(SearchTabHelper);
 
 namespace {
@@ skipping 151 matching lines @@
     // startup or from the user opening a new tab, and if we wait until later,
     // it won't correctly detect this case.
     NTPUserDataLogger::GetOrCreateFromWebContents(web_contents_);
   }
 }
 
 void SearchTabHelper::OnTabDeactivated() {
   ipc_router_.OnTabDeactivated();
 }
 
+void SearchTabHelper::DidStartNavigationToPendingEntry(
+    const GURL& url,
+    content::ReloadType reload_type) {
+  // TODO(jam): delete this method once PlzNavigate is turned on by default.
+  // When PlzNavigate is enabled, DidStartNavigation is called early enough such
+  // that there's no flickering. However when PlzNavigate is disabled,
+  // DidStartNavigation is called too late and "Untitled" shows up momentarily.
+  // The fix is to override this deprecated callback for the non-PlzNavigate
+  // case.

[Marc Treib, 2017/01/20 09:26:47]

Is there a bug you could reference here?
(I'm seeing myself a few months in the future, wondering whether it's safe to
delete this now)

[jam, 2017/01/20 15:11:35]

When PlzNavigate is turned on, we will delete this method and all usages in the
codebase (i.e. no need to label each usage).

+  if (content::IsBrowserSideNavigationEnabled())
+    return;
+
+  if (search::IsNTPURL(url, profile())) {

[Marc Treib, 2017/01/20 09:26:47]

Put this block into a new helper, so it doesn't have to be repeated?

[jam, 2017/01/20 15:11:35]

given that this is only 3 lines of code, I thought it's ok to have it repeated.
Otherwise if I create a function for that now, it'll get folded back in a few
months anyways when this code is removed.

+    // Set the title on any pending entry corresponding to the NTP. This
+    // prevents any flickering of the tab title.
+    content::NavigationEntry* entry =
+        web_contents_->GetController().GetPendingEntry();
+    if (entry) {
+      web_contents_->UpdateTitleForEntry(
+          entry, l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE));
+    }
+  }
+}
+
 void SearchTabHelper::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
+  if (!content::IsBrowserSideNavigationEnabled())

[Marc Treib, 2017/01/20 09:26:47]

Once the non-PlzNavigate hack above is gone, this check needs to be removed too,
right? Please add a comment saying that.

[jam, 2017/01/20 15:11:35]

(same as first comment)

+    return;
+
   if (!navigation_handle->IsInMainFrame() || navigation_handle->IsSamePage())
     return;
 
   if (search::IsNTPURL(navigation_handle->GetURL(), profile())) {
     // Set the title on any pending entry corresponding to the NTP. This
     // prevents any flickering of the tab title.
     content::NavigationEntry* entry =
         web_contents_->GetController().GetPendingEntry();
     if (entry) {
       web_contents_->UpdateTitleForEntry(
@@ skipping 275 matching lines @@
 
 bool SearchTabHelper::IsInputInProgress() const {
   OmniboxView* omnibox = GetOmniboxView();
   return !model_.mode().is_ntp() && omnibox &&
       omnibox->model()->focus_state() == OMNIBOX_FOCUS_VISIBLE;
 }
 
 OmniboxView* SearchTabHelper::GetOmniboxView() const {
   return delegate_ ? delegate_->GetOmniboxView() : NULL;
 }