Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(81)

Issue 8879045: Don't close ExtensionPopups on child window focus (Win, non-Aura). (Closed)

Created:
9 years ago by msw
Modified:
9 years ago
Reviewers:
Matt Perry, sky
CC:
chromium-reviews, jstritar+watch_chromium.org, Aaron Boodman, mihaip+watch_chromium.org, tfarina
Visibility:
Public.

Description

Don't close ExtensionPopups on child window focus (Win, non-Aura). Fixes crash on JS dialog popups from the extension bubbles. Check for child HWND in WidgetFocusChangeListener::OnNativeFocusChange. Code from BubbleWidget in browser_bubble_win.cc (crrev.com/112278) See Issue 106958 for Aura, the dialog&bubble close early without crashing. BUG=106723, 106958 TEST=The js alert extension attached to the issue doesn't crash, popup works on non-aura. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113936

Patch Set 1 #

Patch Set 2 : Skip all of OnNativeFocusChange in Aura. #

Patch Set 3 : Fix bad typo. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -3 lines) Patch
M chrome/browser/ui/views/extensions/extension_popup.h View 2 chunks +8 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/extensions/extension_popup.cc View 1 2 3 chunks +29 lines, -0 lines 0 comments Download
M ui/views/focus/widget_focus_manager.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 8 (0 generated)
msw
Please take a look; thanks! I'll fix Aura in a followup for crbug.com/106958; not sure ...
9 years ago (2011-12-09 09:12:20 UTC) #1
Matt Perry
lgtm, thanks!
9 years ago (2011-12-09 19:51:58 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/8879045/8001
9 years ago (2011-12-10 00:03:53 UTC) #3
commit-bot: I haz the power
Presubmit check for 8879045-8001 failed and returned exit status 1. Running presubmit commit checks ...
9 years ago (2011-12-10 00:03:56 UTC) #4
msw
+Sky for OWNERS ui/views/focus/widget_focus_manager.cc
9 years ago (2011-12-10 00:12:16 UTC) #5
sky
LGTM
9 years ago (2011-12-10 00:19:58 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/8879045/8001
9 years ago (2011-12-10 00:23:48 UTC) #7
commit-bot: I haz the power
9 years ago (2011-12-10 03:57:34 UTC) #8
Change committed as 113936

Powered by Google App Engine
This is Rietveld 408576698