Revert "Focus tab earlier for browser-initiated, //chrome-layer navigations."

This reverts commit 7a8180fa.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 739179 as the
culprit for flakes in the build cycles as shown on:
https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vN2E4MTgwZmFiYzQ0MGE0MWIzMTI2Y2UzNjc1ZmI1ZDg4MTRmYjgxNQw

Sample Failed Build: https://ci.chromium.org/b/8889148211576341952

Sample Failed Step: interactive_ui_tests

Sample Flaky Test: BrowserFocusTest.NavigateFromOmnibox

Original change's description:
> Focus tab earlier for browser-initiated, //chrome-layer navigations.
> 
> This CL changes how navigations result in focusing the tab contents:
> 
> 1. Browser-initiated navigations:
>    - Before this CL: Focus done in FocusTabAfterNavigationHelper
>      which covers both OpenURL and BeginNavigation code paths.
>    - After this CL: Focus done in Browser::UpdateUIForNavigationInTab
>      (similarly to pre-r737502) which covers only the OpenURL code path.
> 
> 2. NTP-initiated navigations:
>    - No change: Focus done in FocusTabAfterNavigationHelper.
> 
> This CL makes sure that the text in the Omnibox is not temporarily
> selected during a navigation, by ensuring that the Omnibox is not
> focused when OmniboxViewViews::Update runs (see also
> https://crbug.com/1048742#c4 and #c5 and #c7).  Regression tests
> are added for this problem.
> 
> This CL also makes sure that DevTools-initiated navigations
> (browser-initiated navigations which go through the BeginNavigation
> path) do not result in focus changes (see also
> https://crbug.com/1048591).
> 
> This CL might also (speculating here) address the performance bugs
> tracked in bugs 1048333, 1048556, 1048565, 1048988, 1048988.
> 
> Fixed: 1048742, 1048591
> Bug: 1048333, 1048556, 1048565, 1048988, 1048988
> Change-Id: Ia00cd9143991c260615663fae06f096e9d7cf0f7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2040234
> Commit-Queue: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#739179}


Change-Id: Ic3ddf4de6a92eecf9175d91b572dacb2b436f371
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1048333, 1048556, 1048565, 1048988, 1048988
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042929
Cr-Commit-Position: refs/heads/master@{#739279}

chrome/browser/ui/browser.cc

@@ -1017,7 +1017,6 @@ bool Browser::CanSaveContents(content::WebContents* web_contents) const {
 
 void Browser::UpdateUIForNavigationInTab(WebContents* contents,
                                          ui::PageTransition transition,
-                                         NavigateParams::WindowAction action,
                                          bool user_initiated) {
   tab_strip_model_->TabNavigating(contents, transition);
 
@@ -1040,17 +1039,6 @@ void Browser::UpdateUIForNavigationInTab(WebContents* contents,
   // the throbber will show the default favicon for a split second when
   // navigating away from the new tab page.
   ScheduleUIUpdate(contents, content::INVALIDATE_TYPE_URL);
-
-  // Navigating contents can take focus (potentially taking it away from other,
-  // currently-focused UI element like the omnibox) if the navigation was
-  // initiated by the user (e.g., via omnibox, bookmarks, etc.).
-  //
-  // Note that focusing contents of NTP-initiated navigations is taken care of
-  // elsewhere - see FocusTabAfterNavigationHelper.
-  if (user_initiated && contents_is_selected &&
-      (window()->IsActive() || action == NavigateParams::SHOW_WINDOW)) {
-    contents->SetInitialFocus();
-  }
 }
 
 void Browser::RegisterKeepAlive() {

chrome/browser/ui/browser.h

@@ -513,7 +513,6 @@ class Browser : public TabStripModelObserver,
   // this Browser. Updates the UI for the start of this navigation.
   void UpdateUIForNavigationInTab(content::WebContents* contents,
                                   ui::PageTransition transition,
-                                  NavigateParams::WindowAction action,
                                   bool user_initiated);
 
   // Used to register a KeepAlive to affect the Chrome lifetime. The KeepAlive

chrome/browser/ui/browser_focus_uitest.cc

@@ -29,7 +29,6 @@
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/url_constants.h"
-#include "chrome/test/base/chrome_test_utils.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -643,43 +642,6 @@ IN_PROC_BROWSER_TEST_F(BrowserFocusTest, DISABLED_FocusAfterCrashedTab) {
   ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER));
 }
 
-// Tests that when omnibox triggers a navigation, then the focus is moved into
-// the current tab.
-IN_PROC_BROWSER_TEST_F(BrowserFocusTest, NavigateFromOmnibox) {
-  const GURL url = embedded_test_server()->GetURL("/title1.html");
-
-  // Focus the Omnibox.
-  ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
-  chrome::FocusLocationBar(browser());
-  OmniboxView* view = browser()->window()->GetLocationBar()->GetOmniboxView();
-
-  // Simulate typing a URL into the omnibox.
-  view->SetUserText(base::UTF8ToUTF16(url.spec()));
-  EXPECT_TRUE(IsViewFocused(VIEW_ID_OMNIBOX));
-  EXPECT_FALSE(view->IsSelectAll());
-
-  // Simulate pressing Enter.
-  content::WebContents* web_contents =
-      chrome_test_utils::GetActiveWebContents(this);
-  content::TestNavigationObserver nav_observer(web_contents, 1);
-  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_RETURN, false,
-                                              false, false, false));
-  // Verify that a navigation has started.
-  EXPECT_TRUE(web_contents->GetController().GetPendingEntry());
-  // Verify that the Omnibox text is not selected - this is a regression test
-  // for https://crbug.com/1048742.
-  EXPECT_FALSE(view->IsSelectAll());
-  // Intentionally not asserting anything about IsViewFocused in this
-  // _intermediate_ state.
-
-  // Wait for the navigation to finish and verify final, steady state.
-  nav_observer.Wait();
-  EXPECT_TRUE(nav_observer.last_navigation_succeeded());
-  EXPECT_EQ(url, web_contents->GetLastCommittedURL());
-  EXPECT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER));
-  EXPECT_FALSE(view->IsSelectAll());
-}
-
 // Tests that when a new tab is opened from the omnibox, the focus is moved from
 // the omnibox for the current tab.
 IN_PROC_BROWSER_TEST_F(BrowserFocusTest, NavigateFromOmniboxIntoNewTab) {

chrome/browser/ui/browser_navigator.cc

@@ -676,8 +676,7 @@ void Navigate(NavigateParams* params) {
        params->disposition == WindowOpenDisposition::CURRENT_TAB)) {
     // The navigation occurred in the source tab.
     params->browser->UpdateUIForNavigationInTab(
-        contents_to_navigate_or_insert, params->transition,
-        params->window_action, user_initiated);
+        contents_to_navigate_or_insert, params->transition, user_initiated);
   } else if (singleton_index == -1) {
     // If some non-default value is set for the index, we should tell the
     // TabStripModel to respect it.

chrome/browser/ui/focus_tab_after_navigation_helper.cc

@@ -50,11 +50,10 @@ bool FocusTabAfterNavigationHelper::ShouldFocusTabContents(
   if (!navigation->IsInMainFrame())
     return false;
 
-  // Browser-initiated navigations (e.g. typing in an omnibox) are taken care of
-  // in Browser::UpdateUIForNavigationInTab.  See also https://crbug.com/1048591
-  // for possible regression risks related to returning |true| here.
+  // Browser-initiated navigations (e.g. typing in an omnibox) should focus
+  // the tab contents.
   if (!navigation->IsRendererInitiated())
-    return false;
+    return true;
 
   // Renderer-initiated navigations shouldn't focus the tab contents, unless the
   // navigation is leaving the NTP.  See also https://crbug.com/1027719.