Commit 24dc1eb6 authored by Hitoshi Yoshida's avatar Hitoshi Yoshida Committed by Commit Bot

Revert "Set initial focus on session restore"

This reverts commit 414b62b0.

Reason for revert: Regresses a test on Mac.

Bug: 1110239

Original change's description:
> Set initial focus on session restore
> 
> Issue:
> - When On startup setting is set to "Continue where you left off",
> the initial focus is not set after launching the browser.
> - Users have to press Tab to focus the window.
> - This affects users who rely on screen readers since the window
> information is not announced on launch (on UIA mode).
> 
> More context:
> - The line calling SetInitialFocus was previously in the code but got
> removed after a startup focus refactor:
> https://chromium-review.googlesource.com/c/chromium/src/+/1313728
> - The refactor added the call to RestoreFocus on
> BrowserView::OnWidgetActivationChanged if the property
> restore_focus_on_activation_ is set to True. This property gets set
> during BrowserView::Show, but only in the case of session restore,
> Show gets called after OnWidgetActivationChanged so RestoreFocus is
> never called.
> - The inverted call stack on session restore is because AddRestoredTab
> on browser_tabrestore calls Activate before showing the window.
> 
> Fix:
> - Similar to what we currently have for MacOS, only call Activate
> in AddRestoredTab if it is not |from_session_restore|. This way Show
> will be called before Activate and thus focus will be set as
> expected.
> - This change is only made on Windows due to a couple of reasons
> 1) crbug.com/1102685 is only present on this platform.
> 2) Making this change on Linux would re-introduce crbug.com/1019048.
> 
> Bug: 1102685
> Change-Id: I6e62ebad7693eb5d908770364ba7c8d755ccaa70
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2284382
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Maria Villarreal <mavill@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#791950}

TBR=xiyuan@chromium.org,sky@chromium.org,fdoray@chromium.org,mavill@microsoft.com

Change-Id: I258891124447cb71b163b35452b84b999857cf96
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1102685
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2322266Reviewed-by: default avatarHitoshi Yoshida <peria@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792176}
parent 4b2bf0f6
...@@ -70,7 +70,6 @@ ...@@ -70,7 +70,6 @@
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h" #include "content/public/browser/render_view_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/common/bindings_policy.h" #include "content/public/common/bindings_policy.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
...@@ -914,11 +913,6 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, Basic) { ...@@ -914,11 +913,6 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, Basic) {
ASSERT_EQ(1u, active_browser_list_->size()); ASSERT_EQ(1u, active_browser_list_->size());
ASSERT_EQ(url2_, ASSERT_EQ(url2_,
new_browser->tab_strip_model()->GetActiveWebContents()->GetURL()); new_browser->tab_strip_model()->GetActiveWebContents()->GetURL());
// Ensure window has initial focus on launch.
EXPECT_TRUE(new_browser->tab_strip_model()
->GetActiveWebContents()
->GetRenderWidgetHostView()
->HasFocus());
GoBack(new_browser); GoBack(new_browser);
ASSERT_EQ(url1_, ASSERT_EQ(url1_,
new_browser->tab_strip_model()->GetActiveWebContents()->GetURL()); new_browser->tab_strip_model()->GetActiveWebContents()->GetURL());
......
...@@ -180,12 +180,11 @@ WebContents* AddRestoredTab( ...@@ -180,12 +180,11 @@ WebContents* AddRestoredTab(
raw_web_contents->WasHidden(); raw_web_contents->WasHidden();
} else { } else {
const bool should_activate = const bool should_activate =
#if defined(OS_WIN) || defined(OS_MACOSX) #if defined(OS_MACOSX)
// Activating a window on another space causes the system to switch to // Activating a window on another space causes the system to switch to
// that space. Since the session restore process shows and activates // that space. Since the session restore process shows and activates
// windows itself, activating windows here should be safe to skip. // windows itself, activating windows here should be safe to skip.
// Cautiously apply only to Windows and MacOS, for now // Cautiously apply only to macOS, for now (https://crbug.com/1019048).
// (https://crbug.com/1019048).
!from_session_restore; !from_session_restore;
#else #else
true; true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment