From 51623832b41997874faf9ee68e9f0430f58e69e9 Mon Sep 17 00:00:00 2001 From: Robert Liao Date: Tue, 17 Apr 2018 20:07:48 +0000 Subject: [PATCH] Revert "Adjust ExtensionPopup Dismissal Behavior" This reverts commit 902d73ba5f92c7f10ea8c07e5839a7143b28d107. Reason for revert: This causes the ExtensionPopup to dismiss if it brings up a child dialog. See http://crbug.com/832623 Fixing this will require adding support to check for descendant NativeWidget activation, which is too big to merge. Original change's description: > Adjust ExtensionPopup Dismissal Behavior > > Before, dismissing the ExtensionPopup when the anchor window received > focus was an arbitrary decision (http://crbug.com/179786) that allowed > the ExtensionPopup to dismiss at most of the right times. However, if > the some other top-level window received activation, the ExtensionPopup > would not dismiss, unlike a typical menu. > > This change adjusts the ExtensionPopup to always dismiss when it loses > activation as long as devtools is not attached. > > When devtools is detached, activation is placed back on the > ExtensionPopup so that the normal dismissal behavior can continue to > work. Failure to receive activation means the ExtensionPopup will not > dismiss until it receives activation at least once. > > BUG=825867 > > Change-Id: I802af281616c66013c370e892953ad2805533728 > Reviewed-on: https://chromium-review.googlesource.com/984404 > Reviewed-by: Scott Violet > Reviewed-by: Elly Fong-Jones > Commit-Queue: Robert Liao > Cr-Commit-Position: refs/heads/master@{#547391} TBR=ellyjones@chromium.org,sky@chromium.org,robliao@chromium.org,rdevlin.cronin@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 832623 Change-Id: I69678b789fdbc56501e67b62dd41467e5f2f8cc9 Reviewed-on: https://chromium-review.googlesource.com/1015181 Commit-Queue: Robert Liao Reviewed-by: Robert Liao Cr-Original-Commit-Position: refs/heads/master@{#551444}(cherry picked from commit f58eccdd24c115ab1f70e310c3823880cdad04a8) Reviewed-on: https://chromium-review.googlesource.com/1015723 Cr-Commit-Position: refs/branch-heads/3396@{#58} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} --- .../ui/cocoa/extensions/extension_popup_views_mac.mm | 4 ++-- chrome/browser/ui/views/extensions/extension_popup.cc | 10 +--------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm b/chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm index e9c24a858a7d..1eb5c296db92 100644 --- a/chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm +++ b/chrome/browser/ui/cocoa/extensions/extension_popup_views_mac.mm @@ -34,8 +34,8 @@ NSNotificationCenter* center = [NSNotificationCenter defaultCenter]; // ObjC id collides with views::View::id(). - ::id token = [center addObserverForName:NSWindowDidResignKeyNotification - object:popup->GetWidget()->GetNativeWindow() + ::id token = [center addObserverForName:NSWindowDidBecomeKeyNotification + object:parent_window queue:nil usingBlock:^(NSNotification* notification) { popup->CloseUnlessUnderInspection(); diff --git a/chrome/browser/ui/views/extensions/extension_popup.cc b/chrome/browser/ui/views/extensions/extension_popup.cc index 2fb2ff6f2041..2dc1859a5e80 100644 --- a/chrome/browser/ui/views/extensions/extension_popup.cc +++ b/chrome/browser/ui/views/extensions/extension_popup.cc @@ -123,14 +123,6 @@ void ExtensionPopup::DevToolsAgentHostDetached( if (host()->host_contents() != agent_host->GetWebContents()) return; inspect_with_devtools_ = false; - // Reset the dismissing logic back to a known state. - // The Chrome top-level window often receives activation when devtools is - // closed. However, because devtools was shown, ExtensionPopup doesn't have - // activation. This means there is no chance for it to dismiss until it - // receives activation again. Checking the anchor window for activation and - // then dismissing would result in the ExtensionPopup dismissing with - // devtools, which is also unexpected. - GetWidget()->Activate(); } ExtensionViewViews* ExtensionPopup::GetExtensionView() { @@ -160,7 +152,7 @@ void ExtensionPopup::AddedToWidget() { void ExtensionPopup::OnWidgetActivationChanged(views::Widget* widget, bool active) { - if (!active && widget == GetWidget()) + if (active && widget == anchor_widget()) CloseUnlessUnderInspection(); }