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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1922813002: Vanquish views::BubbleDelegate{View,} (Closed)

Created:
4 years, 7 months ago by Evan Stade
Modified:
4 years, 7 months ago
Reviewers:
msw, sky
CC:
chromium-reviews, msramek+watch_chromium.org, msw+watch_chromium.org, sadrul, yusukes+watch_chromium.org, nona+watch_chromium.org, rouslan+autofill_chromium.org, tfarina, shuchen+watch_chromium.org, raymes+watch_chromium.org, jdonnelly+autofillwatch_chromium.org, bondd+autofillwatch_chromium.org, groby+bubble_chromium.org, oshima+watch_chromium.org, estade+watch_chromium.org, vabr+watchlistautofill_chromium.org, kalyank, markusheintz_, stevenjb+watch_chromium.org, hcarmona+bubble_chromium.org, rouslan+bubble_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Vanquish views::BubbleDelegate{View,} BUG=585312 Committed: https://crrev.com/255096216060f960a3bd65e2f565182a56beb3ec Cr-Commit-Position: refs/heads/master@{#389915}

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : fix unit tests #

Total comments: 9

Patch Set 4 : fix other test, respond to comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -953 lines) Patch
M ash/wm/immersive_fullscreen_controller.cc View 1 2 3 6 chunks +7 lines, -34 lines 0 comments Download
M ash/wm/immersive_fullscreen_controller_unittest.cc View 1 2 3 10 chunks +35 lines, -25 lines 0 comments Download
M chrome/browser/ui/views/autofill/save_card_bubble_views.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/location_bar/zoom_bubble_view.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/views/toolbar/browser_actions_container.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/website_settings/permissions_bubble_view.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/chromeos/ime/mode_indicator_view.cc View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
D ui/views/bubble/bubble_delegate.h View 1 chunk +0 lines, -220 lines 0 comments Download
D ui/views/bubble/bubble_delegate.cc View 1 chunk +0 lines, -335 lines 0 comments Download
D ui/views/bubble/bubble_delegate_unittest.cc View 1 chunk +0 lines, -323 lines 0 comments Download
M ui/views/views.gyp View 2 chunks +0 lines, -3 lines 0 comments Download
M ui/views/widget/widget_delegate.h View 2 chunks +0 lines, -2 lines 0 comments Download
M ui/views/widget/widget_delegate.cc View 2 chunks +0 lines, -5 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
Evan Stade
https://codereview.chromium.org/1922813002/diff/40001/ash/wm/immersive_fullscreen_controller_unittest.cc File ash/wm/immersive_fullscreen_controller_unittest.cc (left): https://codereview.chromium.org/1922813002/diff/40001/ash/wm/immersive_fullscreen_controller_unittest.cc#oldcode677 ash/wm/immersive_fullscreen_controller_unittest.cc:677: TEST_F(ImmersiveFullscreenControllerTest, MAYBE_EndRevealViaGesture) { note that this test failed on ...
4 years, 7 months ago (2016-04-26 17:11:40 UTC) #2
msw
lgtm with nits https://codereview.chromium.org/1922813002/diff/40001/ash/wm/immersive_fullscreen_controller.cc File ash/wm/immersive_fullscreen_controller.cc (right): https://codereview.chromium.org/1922813002/diff/40001/ash/wm/immersive_fullscreen_controller.cc#newcode224 ash/wm/immersive_fullscreen_controller.cc:224: for (std::set<aura::Window*>::const_iterator it = bubbles_.begin(); optional ...
4 years, 7 months ago (2016-04-26 18:07:30 UTC) #3
sky
LGTM https://codereview.chromium.org/1922813002/diff/40001/ash/wm/immersive_fullscreen_controller_unittest.cc File ash/wm/immersive_fullscreen_controller_unittest.cc (right): https://codereview.chromium.org/1922813002/diff/40001/ash/wm/immersive_fullscreen_controller_unittest.cc#newcode39 ash/wm/immersive_fullscreen_controller_unittest.cc:39: TestBubbleDialogDelegate(views::View* anchor) nit: explicit
4 years, 7 months ago (2016-04-26 18:14:04 UTC) #4
Evan Stade
https://codereview.chromium.org/1922813002/diff/40001/ash/wm/immersive_fullscreen_controller.cc File ash/wm/immersive_fullscreen_controller.cc (right): https://codereview.chromium.org/1922813002/diff/40001/ash/wm/immersive_fullscreen_controller.cc#newcode224 ash/wm/immersive_fullscreen_controller.cc:224: for (std::set<aura::Window*>::const_iterator it = bubbles_.begin(); On 2016/04/26 18:07:30, msw ...
4 years, 7 months ago (2016-04-26 21:23:01 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1922813002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1922813002/60001
4 years, 7 months ago (2016-04-26 21:23:27 UTC) #8
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 7 months ago (2016-04-26 22:19:16 UTC) #9
commit-bot: I haz the power
4 years, 7 months ago (2016-04-26 22:21:33 UTC) #11
Message was sent while issue was closed. 
Patchset 4 (id:??) landed as
https://crrev.com/255096216060f960a3bd65e2f565182a56beb3ec
Cr-Commit-Position: refs/heads/master@{#389915}

Powered by Google App Engine
This is Rietveld 408576698