Skip to content

Commit

Permalink
Revert "Adjust ExtensionPopup Dismissal Behavior"
Browse files Browse the repository at this point in the history
This reverts commit 902d73b.

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 <[email protected]>
> Reviewed-by: Elly Fong-Jones <[email protected]>
> Commit-Queue: Robert Liao <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#547391}

[email protected],[email protected],[email protected],[email protected]

# 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 <[email protected]>
Reviewed-by: Robert Liao <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#551444}(cherry picked from commit f58eccd)
Reviewed-on: https://chromium-review.googlesource.com/1015723
Cr-Commit-Position: refs/branch-heads/3396@{#58}
Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}
  • Loading branch information
Robert Liao committed Apr 17, 2018
1 parent 11ecb03 commit 5162383
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 1 addition & 9 deletions chrome/browser/ui/views/extensions/extension_popup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
}

Expand Down

0 comments on commit 5162383

Please sign in to comment.