Commit a6525084 authored by Leonard Grey's avatar Leonard Grey Committed by Commit Bot

Reland "Mac: windows always display as key if a child is key"

This reverts commit 34d7e5df.

Reason for revert: Speculative fix in patchset 2

Original change's description:
> Revert "Mac: windows always display as key if a child is key"
>
> This reverts commit e8438fe0.
>
> Reason for revert: https://crbug.com/1050430
>
> Original change's description:
> > Mac: windows always display as key if a child is key
> >
> > On other platforms, user expectation is that a parent window will display
> > as inactive when a modal is showing. This isn't the case on Mac.
> >
> > Additionally, the code that passes key commands up to the parent window
> > uses this status to determine whether to pass keys from child windows up.
> >
> > This change (hopefully) shows a window as active iff a child is active.
> > It plays well with the current code that handles this for bubbles because
> > PaintAsActive is now reference counted.
> >
> > Still outstanding: making a non-key child window key *does* cause the
> > browser frame to be drawn as key and *does* handle hotkeys correctly,
> > but does *not* fix the traffic lights until the browser window has been
> > interacted with.
> >
> > Bug: 1046540
> > Change-Id: I1629e66457c42b268c2283a8dbeb6823c66b1111
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033432
> > Commit-Queue: Leonard Grey <lgrey@chromium.org>
> > Reviewed-by: ccameron <ccameron@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#739444}
>
> TBR=ccameron@chromium.org,lgrey@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 1046540
> Change-Id: Ic25e1562df10809be9cdc9cd886fbf794b96b3c6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2047085
> Reviewed-by: Leonard Grey <lgrey@chromium.org>
> Commit-Queue: Leonard Grey <lgrey@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#739878}

TBR=ccameron@chromium.org,lgrey@chromium.org

Change-Id: I6ac87880427b118ab06b822f7372e3b345dab86e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1046540
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2048144Reviewed-by: default avatarLeonard Grey <lgrey@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740447}
parent 63f8588a
...@@ -557,7 +557,13 @@ TEST_F(BubbleDialogDelegateViewTest, VisibleAnchorChanges) { ...@@ -557,7 +557,13 @@ TEST_F(BubbleDialogDelegateViewTest, VisibleAnchorChanges) {
Widget* bubble_widget = Widget* bubble_widget =
BubbleDialogDelegateView::CreateBubble(bubble_delegate); BubbleDialogDelegateView::CreateBubble(bubble_delegate);
bubble_widget->Show(); bubble_widget->Show();
#if defined(OS_MACOSX)
// All child widgets make the parent paint as active on Mac.
// See https://crbug.com/1046540
EXPECT_TRUE(anchor_widget->ShouldPaintAsActive());
#else
EXPECT_FALSE(anchor_widget->ShouldPaintAsActive()); EXPECT_FALSE(anchor_widget->ShouldPaintAsActive());
#endif // defined(OS_MACOSX)
bubble_delegate->SetAnchorView(anchor_widget->GetContentsView()); bubble_delegate->SetAnchorView(anchor_widget->GetContentsView());
EXPECT_TRUE(anchor_widget->ShouldPaintAsActive()); EXPECT_TRUE(anchor_widget->ShouldPaintAsActive());
......
...@@ -267,6 +267,8 @@ class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate, ...@@ -267,6 +267,8 @@ class VIEWS_EXPORT NativeWidgetMac : public internal::NativeWidgetPrivate,
FocusManager* focus_manager_ = nullptr; FocusManager* focus_manager_ = nullptr;
std::unique_ptr<ui::InputMethod> input_method_; std::unique_ptr<ui::InputMethod> input_method_;
std::unique_ptr<ZoomFocusMonitor> zoom_focus_monitor_; std::unique_ptr<ZoomFocusMonitor> zoom_focus_monitor_;
// Held while this widget is active if it's a child.
std::unique_ptr<Widget::PaintAsActiveLock> parent_key_lock_;
DISALLOW_COPY_AND_ASSIGN(NativeWidgetMac); DISALLOW_COPY_AND_ASSIGN(NativeWidgetMac);
}; };
......
...@@ -157,9 +157,18 @@ void NativeWidgetMac::OnWindowKeyStatusChanged( ...@@ -157,9 +157,18 @@ void NativeWidgetMac::OnWindowKeyStatusChanged(
if (is_key) { if (is_key) {
widget->OnNativeFocus(); widget->OnNativeFocus();
widget->GetFocusManager()->RestoreFocusedView(); widget->GetFocusManager()->RestoreFocusedView();
if (NativeWidgetMacNSWindowHost* parent_host = ns_window_host_->parent()) {
// Unclear under what circumstances this would be null, but speculatively
// working around https://crbug/1050430
if (Widget* top_widget =
parent_host->native_widget_mac()->GetTopLevelWidget()) {
parent_key_lock_ = top_widget->LockPaintAsActive();
}
}
} else { } else {
widget->OnNativeBlur(); widget->OnNativeBlur();
widget->GetFocusManager()->StoreFocusedView(true); widget->GetFocusManager()->StoreFocusedView(true);
parent_key_lock_.reset();
} }
} }
...@@ -800,6 +809,7 @@ void NativeWidgetMac::OnNativeViewHierarchyWillChange() { ...@@ -800,6 +809,7 @@ void NativeWidgetMac::OnNativeViewHierarchyWillChange() {
// listeners. // listeners.
if (!GetWidget()->is_top_level()) if (!GetWidget()->is_top_level())
SetFocusManager(nullptr); SetFocusManager(nullptr);
parent_key_lock_.reset();
} }
void NativeWidgetMac::OnNativeViewHierarchyChanged() { void NativeWidgetMac::OnNativeViewHierarchyChanged() {
......
...@@ -199,15 +199,21 @@ TEST_F(NativeWidgetMacInteractiveUITest, ParentWindowTrafficLights) { ...@@ -199,15 +199,21 @@ TEST_F(NativeWidgetMacInteractiveUITest, ParentWindowTrafficLights) {
EXPECT_TRUE(button); EXPECT_TRUE(button);
NSData* active_button_image = ViewAsTIFF(button); NSData* active_button_image = ViewAsTIFF(button);
EXPECT_TRUE(active_button_image); EXPECT_TRUE(active_button_image);
EXPECT_TRUE(parent_widget->ShouldPaintAsActive());
// Pop open a bubble on the parent Widget. When the visibility of Bubbles with
// an anchor View changes, BubbleDialogDelegateView::HandleVisibilityChanged() // If a child widget is key, the parent should paint as active.
// updates Widget::ShouldPaintAsActive() accordingly. Widget* child_widget = new Widget;
ShowKeyWindow(BubbleDialogDelegateView::CreateBubble( Widget::InitParams params =
new TestBubbleView(parent_widget))); CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS);
params.parent = parent_widget->GetNativeView();
child_widget->Init(std::move(params));
child_widget->SetContentsView(new View);
child_widget->Show();
NSWindow* child = child_widget->GetNativeWindow().GetNativeNSWindow();
// Ensure the button instance is still valid. // Ensure the button instance is still valid.
EXPECT_EQ(button, [parent standardWindowButton:NSWindowCloseButton]); EXPECT_EQ(button, [parent standardWindowButton:NSWindowCloseButton]);
EXPECT_TRUE(parent_widget->ShouldPaintAsActive());
// Parent window should still be main, and have its traffic lights active. // Parent window should still be main, and have its traffic lights active.
EXPECT_TRUE([parent isMainWindow]); EXPECT_TRUE([parent isMainWindow]);
...@@ -226,10 +232,18 @@ TEST_F(NativeWidgetMacInteractiveUITest, ParentWindowTrafficLights) { ...@@ -226,10 +232,18 @@ TEST_F(NativeWidgetMacInteractiveUITest, ParentWindowTrafficLights) {
ShowKeyWindow(other_widget); ShowKeyWindow(other_widget);
EXPECT_FALSE([parent isMainWindow]); EXPECT_FALSE([parent isMainWindow]);
EXPECT_FALSE([parent isKeyWindow]); EXPECT_FALSE([parent isKeyWindow]);
EXPECT_FALSE(parent_widget->ShouldPaintAsActive());
EXPECT_TRUE([button isEnabled]); EXPECT_TRUE([button isEnabled]);
NSData* inactive_button_image = ViewAsTIFF(button); NSData* inactive_button_image = ViewAsTIFF(button);
EXPECT_FALSE([active_button_image isEqualToData:inactive_button_image]); EXPECT_FALSE([active_button_image isEqualToData:inactive_button_image]);
// Focus the child again and assert the parent once again paints as active.
[child makeKeyWindow];
EXPECT_TRUE(parent_widget->ShouldPaintAsActive());
EXPECT_TRUE([child isKeyWindow]);
EXPECT_FALSE([parent isKeyWindow]);
child_widget->CloseNow();
other_widget->CloseNow(); other_widget->CloseNow();
parent_widget->CloseNow(); parent_widget->CloseNow();
} }
......
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