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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 613563002: Make WindowState::CanMinimize() take into account WidgetDelegate::CanMinimize() (Closed)

Created:
6 years, 2 months ago by pkotwicz
Modified:
6 years, 2 months ago
Reviewers:
oshima, sky
CC:
chromium-reviews, tdanderson+views_chromium.org, ben+aura_chromium.org, tfarina, sadrul, kalyank, ben+views_chromium.org, ben+ash_chromium.org, jackhou1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Make WindowState::CanMinimize() take into account WidgetDelegate::CanMinimize() This has the side effect of disallowing minimizing bubbles (e.g. the bookmark bubble) via a gesture BUG=328241 TEST=Manual, see bug Committed: https://crrev.com/ffacfd8c2841480e1e5775c124b63b351bddca69 Cr-Commit-Position: refs/heads/master@{#297732}

Patch Set 1 : #

Total comments: 2

Patch Set 2 : #

Total comments: 4

Patch Set 3 : #

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+86 lines, -71 lines) Patch
M ash/frame/caption_buttons/frame_caption_button_container_view.h View 1 chunk +1 line, -11 lines 0 comments Download
M ash/frame/caption_buttons/frame_caption_button_container_view.cc View 2 chunks +2 lines, -3 lines 0 comments Download
M ash/frame/caption_buttons/frame_caption_button_container_view_unittest.cc View 8 chunks +21 lines, -19 lines 0 comments Download
M ash/frame/caption_buttons/frame_size_button_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M ash/frame/custom_frame_view_ash.cc View 1 chunk +1 line, -6 lines 0 comments Download
M ash/frame/default_header_painter_unittest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M ash/test/ash_test_base.cc View 1 chunk +1 line, -0 lines 0 comments Download
M ash/wm/panels/panel_frame_view.cc View 1 chunk +1 line, -2 lines 0 comments Download
M ash/wm/system_gesture_event_filter_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M ash/wm/toplevel_window_event_handler_unittest.cc View 2 chunks +4 lines, -13 lines 0 comments Download
M ash/wm/window_state.cc View 1 chunk +1 line, -9 lines 0 comments Download
A chrome/browser/chromeos/login/ui/login_web_dialog_browsertest.cc View 1 2 1 chunk +37 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M chrome/chrome_tests.gypi View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M ui/aura/client/aura_constants.h View 1 chunk +3 lines, -0 lines 0 comments Download
M ui/aura/client/aura_constants.cc View 1 chunk +1 line, -0 lines 0 comments Download
M ui/views/widget/desktop_aura/desktop_native_widget_aura.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M ui/views/widget/native_widget_aura.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M ui/views/widget/native_widget_aura_unittest.cc View 2 chunks +3 lines, -2 lines 0 comments Download

Messages

Total messages: 21 (7 generated)
pkotwicz
Oshima can you please take a look?
6 years, 2 months ago (2014-09-27 23:22:52 UTC) #4
pkotwicz
6 years, 2 months ago (2014-09-27 23:23:09 UTC) #5
oshima
nice change. one question. https://codereview.chromium.org/613563002/diff/60001/ash/wm/window_state.cc File ash/wm/window_state.cc (left): https://codereview.chromium.org/613563002/diff/60001/ash/wm/window_state.cc#oldcode162 ash/wm/window_state.cc:162: return false; Loosk like I ...
6 years, 2 months ago (2014-09-29 18:32:20 UTC) #7
pkotwicz
Oshima, can you please take another look? This CL is now dependant on the trivial ...
6 years, 2 months ago (2014-09-30 14:48:22 UTC) #9
oshima
lgtm with one q in desktop aura. https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc File ui/views/widget/desktop_aura/desktop_native_widget_aura.cc (right): https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc#newcode940 ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:940: GetWidget()->widget_delegate()->CanMinimize()); q: ...
6 years, 2 months ago (2014-09-30 16:43:07 UTC) #10
pkotwicz
sky@ for chrome/ and ui/ OWNERS https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc File ui/views/widget/desktop_aura/desktop_native_widget_aura.cc (right): https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc#newcode940 ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:940: GetWidget()->widget_delegate()->CanMinimize()); It is ...
6 years, 2 months ago (2014-10-01 14:45:03 UTC) #11
pkotwicz
sky@ for OWNERS for real
6 years, 2 months ago (2014-10-01 17:21:12 UTC) #13
oshima
https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc File ui/views/widget/desktop_aura/desktop_native_widget_aura.cc (right): https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc#newcode940 ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:940: GetWidget()->widget_delegate()->CanMinimize()); On 2014/10/01 14:45:03, pkotwicz wrote: > It is ...
6 years, 2 months ago (2014-10-01 17:26:55 UTC) #14
sky
LGTM https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc File ui/views/widget/desktop_aura/desktop_native_widget_aura.cc (right): https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc#newcode940 ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:940: GetWidget()->widget_delegate()->CanMinimize()); On 2014/10/01 17:26:55, oshima wrote: > On ...
6 years, 2 months ago (2014-10-01 20:00:27 UTC) #15
oshima
On 2014/10/01 20:00:27, sky wrote: > LGTM > > https://codereview.chromium.org/613563002/diff/100001/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc > File ui/views/widget/desktop_aura/desktop_native_widget_aura.cc (right): > ...
6 years, 2 months ago (2014-10-01 20:16:22 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/613563002/120001
6 years, 2 months ago (2014-10-01 21:21:58 UTC) #18
commit-bot: I haz the power
Committed patchset #3 (id:120001) as 8598038509b25e7fe1098db1d1ffd328aecb9fe3
6 years, 2 months ago (2014-10-01 23:03:10 UTC) #19
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/ffacfd8c2841480e1e5775c124b63b351bddca69 Cr-Commit-Position: refs/heads/master@{#297732}
6 years, 2 months ago (2014-10-01 23:03:51 UTC) #20
tapted
6 years, 2 months ago (2014-10-02 00:42:15 UTC) #21
Message was sent while issue was closed. 
A revert of this CL (patchset #3 id:120001) has been created in
https://codereview.chromium.org/623613004/ by tapted@chromium.org.

The reason for reverting is: Suspected for failures on Linux ChromiumOS Tests
(dbg)(1)
link:
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...

browser_test LoginWebDialogTest.CannotMinimize failing with:
[12496:12496:1001/170357:FATAL:login_web_dialog.cc(148)] Check failed: false.
#2 0x0000026179dd chromeos::LoginWebDialog::OnCloseContents()
#3 0x7f542cba9b00 views::WebDialogView::OnCloseContents()
/* snip */
#24 0x7f5422726679 aura::Window::~Window()
#25 0x7f54244db0f8 ash::RootWindowController::CloseChildWindows()
#26 0x7f5424444bad ash::DisplayController::CloseChildWindows()
#27 0x7f542452db9b ash::Shell::~Shell()
#28 0x7f542452e869 ash::Shell::~Shell()
#29 0x7f542452a16f ash::Shell::DeleteInstance()
#30 0x000002dac13b chrome::CloseAsh()
#31 0x000002a0e111 ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun()
.

Powered by Google App Engine
This is Rietveld 408576698