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

Issue 10920028: Reland: Fix and expand upon BubbleDelegateTests. (Closed)

Created:
8 years, 3 months ago by msw
Modified:
8 years, 3 months ago
CC:
chromium-reviews, tfarina, alicet1, msw+watch_chromium.org, DaveMoore
Visibility:
Public.

Description

Reland: Fix and expand upon BubbleDelegateTests. This is a continuation of https://codereview.chromium.org/10889014 That CL was reverted due to test failures in linux_chromeos and win_aura bots: http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%281%29/builds/10785/steps/views_unittests/logs/stdio http://build.chromium.org/p/chromium/builders/Win%20Aura/builds/20590/steps/views_unittests/logs/stdio This revised CL adds a workaround hack for Aura to activate an unrelated Widget before closing the anchor/owner widgets, so that aura::test::TestActivationClient::OnWindowDestroyed doesn't try to activate destroyed windows. Original CL description: =============================================== Fix and expand upon BubbleDelegateTests. BubbleDelegateTest.ResetAnchorWidget was previously failing and crashing because of it's widget ownership pattern and lack of close_on_deactivate. Add TestWidgetObserver to observe bubbles closing. Ensure the bubble widget closes as expected in each test. BUG=none TEST=unit tests pass Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154558

Patch Set 1 #

Patch Set 2 : Use an activation hack to workaround TestActivationClient. #

Patch Set 3 : Check that the Bubble doesn't close on aura activation hack. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+113 lines, -28 lines) Patch
M ui/views/bubble/bubble_delegate_unittest.cc View 1 2 3 chunks +113 lines, -28 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
msw
Hey Ben, please take a look; any advice on how to nix the hack is ...
8 years, 3 months ago (2012-08-31 02:23:32 UTC) #1
Ben Goodger (Google)
lgtm
8 years, 3 months ago (2012-08-31 22:40:57 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/10920028/3002
8 years, 3 months ago (2012-08-31 22:43:38 UTC) #3
commit-bot: I haz the power
8 years, 3 months ago (2012-09-01 01:15:15 UTC) #4
Change committed as 154558

Powered by Google App Engine
This is Rietveld 408576698