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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 286733002: [Refactor] Consolidate the logic for whether a widget can be activated. (Closed)

Created:
6 years, 7 months ago by pkotwicz
Modified:
6 years, 7 months ago
Reviewers:
sky
CC:
chromium-reviews, sadrul, nkostylev+watch_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, alicet1, ben+mojo_chromium.org, qsr+mojo_chromium.org, tdanderson+views_chromium.org, msw+watch_chromium.org, stevenjb+watch_chromium.org, abarth-chromium, kalyank, tdanderson+overview_chromium.org, dcheng, oshima+watch_chromium.org, ben+views_chromium.org, tfarina, Aaron Boodman, ben+ash_chromium.org, darin (slow to review), James Su, davemoore+watch_chromium.org
Visibility:
Public.

Description

[Refactor] Consolidate the logic for whether a widget can be activated. This is a first step towards giving tooltips their own WindowTreeHosts on Linux. BUG=353533 TEST=None Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270597

Patch Set 1 : #

Total comments: 3

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+54 lines, -61 lines) Patch
M ash/accelerators/exit_warning_handler.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/display/shared_display_edge_indicator.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M ash/magnifier/partial_magnification_controller.cc View 1 chunk +1 line, -1 line 0 comments Download
M ash/shelf/shelf_widget.cc View 1 chunk +1 line, -1 line 0 comments Download
M ash/sticky_keys/sticky_keys_overlay.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/tooltips/tooltip_controller_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/touch/touch_observer_hud.cc View 1 chunk +1 line, -1 line 0 comments Download
M ash/wm/dock/docked_window_layout_manager.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/wm/drag_window_controller.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M ash/wm/gestures/long_press_affordance_handler.cc View 1 chunk +1 line, -1 line 0 comments Download
M ash/wm/maximize_mode/workspace_backdrop_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download
M ash/wm/overview/scoped_window_copy.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/wm/overview/window_overview.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/wm/overview/window_selector_item.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/wm/overview/window_selector_window.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/wm/panels/panel_layout_manager.cc View 2 chunks +0 lines, -2 lines 0 comments Download
M ash/wm/workspace/multi_window_resize_controller.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ash/wm/workspace/phantom_window_controller.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/chromeos/ui/idle_app_name_notification_view.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/first_run/try_chrome_dialog_view.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/fullscreen_exit_bubble_views.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/views/screen_capture_notification_ui_views.cc View 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/ui/views/tabs/tab_strip.cc View 1 chunk +0 lines, -1 line 0 comments Download
M mojo/examples/launcher/launcher.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/views/bubble/bubble_delegate.cc View 1 chunk +2 lines, -1 line 0 comments Download
M ui/views/touchui/touch_selection_controller_impl.cc View 1 chunk +0 lines, -1 line 0 comments Download
M ui/views/widget/desktop_aura/desktop_native_widget_aura.h View 1 chunk +0 lines, -3 lines 0 comments Download
M ui/views/widget/desktop_aura/desktop_native_widget_aura.cc View 3 chunks +4 lines, -3 lines 0 comments Download
M ui/views/widget/native_widget_aura.h View 1 chunk +0 lines, -3 lines 0 comments Download
M ui/views/widget/native_widget_aura.cc View 5 chunks +3 lines, -7 lines 0 comments Download
M ui/views/widget/native_widget_aura_unittest.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M ui/views/widget/widget.h View 3 chunks +12 lines, -1 line 0 comments Download
M ui/views/widget/widget.cc View 6 chunks +20 lines, -9 lines 0 comments Download
M ui/views/widget/widget_hwnd_utils.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M ui/views/widget/widget_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 18 (0 generated)
pkotwicz
Scott, PTAL https://codereview.chromium.org/286733002/diff/20001/ui/views/widget/widget.cc File ui/views/widget/widget.cc (right): https://codereview.chromium.org/286733002/diff/20001/ui/views/widget/widget.cc#newcode140 ui/views/widget/widget.cc:140: keep_on_top(type == TYPE_MENU || type == TYPE_DRAG), ...
6 years, 7 months ago (2014-05-14 00:44:05 UTC) #1
sky
https://codereview.chromium.org/286733002/diff/20001/ash/wm/drag_window_controller.cc File ash/wm/drag_window_controller.cc (right): https://codereview.chromium.org/286733002/diff/20001/ash/wm/drag_window_controller.cc#newcode69 ash/wm/drag_window_controller.cc:69: params.activatable = views::Widget::InitParams::ACTIVATABLE_NO; How come you're explicit here, but ...
6 years, 7 months ago (2014-05-14 14:06:30 UTC) #2
pkotwicz
Scott, can you please take another look? https://codereview.chromium.org/286733002/diff/20001/ash/wm/drag_window_controller.cc File ash/wm/drag_window_controller.cc (right): https://codereview.chromium.org/286733002/diff/20001/ash/wm/drag_window_controller.cc#newcode69 ash/wm/drag_window_controller.cc:69: params.activatable = ...
6 years, 7 months ago (2014-05-14 15:24:37 UTC) #3
sky
LGTM
6 years, 7 months ago (2014-05-14 18:25:00 UTC) #4
pkotwicz
The CQ bit was checked by pkotwicz@chromium.org
6 years, 7 months ago (2014-05-14 18:36:37 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/286733002/40001
6 years, 7 months ago (2014-05-14 18:37:29 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-14 21:47:47 UTC) #7
pkotwicz
The CQ bit was unchecked by pkotwicz@chromium.org
6 years, 7 months ago (2014-05-14 21:51:38 UTC) #8
pkotwicz
The CQ bit was checked by pkotwicz@chromium.org
6 years, 7 months ago (2014-05-14 21:55:33 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/286733002/50001
6 years, 7 months ago (2014-05-14 21:56:06 UTC) #10
pkotwicz
The CQ bit was unchecked by pkotwicz@chromium.org
6 years, 7 months ago (2014-05-14 21:58:36 UTC) #11
pkotwicz
The CQ bit was checked by pkotwicz@chromium.org
6 years, 7 months ago (2014-05-14 22:02:15 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/286733002/70001
6 years, 7 months ago (2014-05-14 22:03:49 UTC) #13
pkotwicz
The CQ bit was unchecked by pkotwicz@chromium.org
6 years, 7 months ago (2014-05-15 01:17:10 UTC) #14
pkotwicz
The CQ bit was checked by pkotwicz@chromium.org
6 years, 7 months ago (2014-05-15 01:19:18 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/286733002/90001
6 years, 7 months ago (2014-05-15 01:20:58 UTC) #16
commit-bot: I haz the power
Change committed as 270597
6 years, 7 months ago (2014-05-15 06:37:46 UTC) #17
pkotwicz
6 years, 7 months ago (2014-05-19 14:14:44 UTC) #18
Message was sent while issue was closed. 
A revert of this CL has been created in
https://codereview.chromium.org/283333010/ by pkotwicz@chromium.org.

The reason for reverting is: This CL was causing problems when the value of
views::Widget::CanActivate() changes crbug.com/374095

I also need to audit the code to find out whether there are any
'non-activatable' widgets which were intended to be activatable.

Powered by Google App Engine
This is Rietveld 408576698