Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(20)

Issue 2700523004: Remove docked windows entirely in M59. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months ago by afakhry
Modified:
6 months, 1 week ago
CC:
asvitkine+watch_chromium.org, chfremer+watch_chromium.org, chromium-apps-reviews_chromium.org, chromium-reviews, dcheng, extensions-reviews_chromium.org, feature-media-reviews_chromium.org, kalyank, mcasas+watch+vc_chromium.org, srahim+watch_chromium.org, tfarina
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove docked windows entirely in M59. After we hid this feature behind a flag in M57, now it's time to remove it completely. This CL removes the feature, its tests, and the newly added flag. BUG=668355 Review-Url: https://codereview.chromium.org/2700523004 Cr-Commit-Position: refs/heads/master@{#458508} Committed: https://chromium.googlesource.com/chromium/src/+/cd7193637823047ec88226019bb1d987215a35db

Patch Set 1 #

Patch Set 2 : Rebase and fix 1 test #

Total comments: 12

Patch Set 3 : Oshima's comments + fix mac compile error #

Patch Set 4 : Rebase #

Patch Set 5 : Yet another Rebase ... #

Total comments: 8

Patch Set 6 : One more rebase #

Patch Set 7 : Delete wrongly added file #

Patch Set 8 : Use panel in test #

Total comments: 4

Patch Set 9 : Rebase #

Total comments: 6

Patch Set 10 : oshima + mfomitchev comments #

Total comments: 30

Patch Set 11 : Rebase and update commit messgae (M59) #

Patch Set 12 : varkha's comments #

Total comments: 3

Patch Set 13 : Rebase #

Total comments: 4

Patch Set 14 : Rebase + Nits #

Total comments: 8

Patch Set 15 : sky's comments #

Total comments: 6

Patch Set 16 : Rebase + Devlin's comments #

Patch Set 17 : Devlin's comments #

Patch Set 18 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+225 lines, -6186 lines) Patch
M ash/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +0 lines, -7 lines 0 comments Download
M ash/accelerators/accelerator_controller_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 5 chunks +11 lines, -215 lines 0 comments Download
M ash/common/accelerators/accelerator_controller.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 5 chunks +13 lines, -15 lines 0 comments Download
M ash/common/accelerators/accelerator_table.h View 1 chunk +2 lines, -2 lines 0 comments Download
M ash/common/accelerators/accelerator_table.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +4 lines, -4 lines 0 comments Download
M ash/common/ash_switches.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +0 lines, -4 lines 0 comments Download
M ash/common/ash_switches.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +0 lines, -9 lines 0 comments Download
M ash/common/shelf/shelf_layout_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +0 lines, -13 lines 0 comments Download
M ash/common/shelf/shelf_layout_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +0 lines, -22 lines 0 comments Download
M ash/common/shelf/shelf_window_watcher_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +0 lines, -40 lines 0 comments Download
M ash/common/wm/default_state.cc View 1 2 3 4 5 6 7 8 9 10 11 12 15 chunks +15 lines, -145 lines 0 comments Download
D ash/common/wm/dock/dock_types.h View 1 chunk +0 lines, -53 lines 0 comments Download
D ash/common/wm/dock/docked_window_layout_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +0 lines, -334 lines 0 comments Download
D ash/common/wm/dock/docked_window_layout_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +0 lines, -1355 lines 0 comments Download
D ash/common/wm/dock/docked_window_layout_manager_observer.h View 1 chunk +0 lines, -39 lines 0 comments Download
D ash/common/wm/dock/docked_window_resizer.h View 1 chunk +0 lines, -101 lines 0 comments Download
D ash/common/wm/dock/docked_window_resizer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +0 lines, -327 lines 0 comments Download
M ash/common/wm/drag_details.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -2 lines 0 comments Download
M ash/common/wm/fullscreen_window_finder.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -4 lines 0 comments Download
M ash/common/wm/lock_layout_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -2 lines 0 comments Download
M ash/common/wm/lock_window_state.cc View 1 chunk +2 lines, -3 lines 0 comments Download
M ash/common/wm/maximize_mode/maximize_mode_window_manager.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -4 lines 0 comments Download
M ash/common/wm/maximize_mode/maximize_mode_window_state.cc View 2 chunks +3 lines, -15 lines 0 comments Download
M ash/common/wm/mru_window_tracker_unittest.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -6 lines 0 comments Download
M ash/common/wm/overview/window_grid.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -2 lines 0 comments Download
M ash/common/wm/panels/panel_layout_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +0 lines, -2 lines 0 comments Download
M ash/common/wm/switchable_windows.cc View 1 chunk +1 line, -2 lines 0 comments Download
M ash/common/wm/window_resizer.cc View 3 chunks +0 lines, -9 lines 0 comments Download
M ash/common/wm/window_state.h View 1 chunk +0 lines, -1 line 0 comments Download
M ash/common/wm/window_state.cc View 3 chunks +1 line, -10 lines 0 comments Download
M ash/common/wm/window_state_observer.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -1 line 0 comments Download
M ash/common/wm/wm_event.h View 2 chunks +6 lines, -8 lines 0 comments Download
M ash/common/wm/wm_types.h View 2 chunks +0 lines, -3 lines 0 comments Download
M ash/common/wm/wm_types.cc View 1 chunk +0 lines, -4 lines 0 comments Download
M ash/common/wm/workspace/workspace_window_resizer.h View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +2 lines, -9 lines 0 comments Download
M ash/common/wm/workspace/workspace_window_resizer.cc View 1 2 3 4 5 6 7 8 9 10 11 11 chunks +24 lines, -87 lines 0 comments Download
M ash/common/wm/workspace_controller.cc View 1 2 2 chunks +13 lines, -22 lines 0 comments Download
M ash/display/display_manager_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +0 lines, -26 lines 0 comments Download
M ash/metrics/user_metrics_recorder.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +7 lines, -12 lines 0 comments Download
M ash/public/cpp/shell_window_ids.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +47 lines, -20 lines 0 comments Download
M ash/public/cpp/shell_window_ids.cc View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M ash/root_window_controller.h View 1 2 3 4 5 6 7 8 9 10 3 chunks +0 lines, -6 lines 0 comments Download
M ash/root_window_controller.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 8 chunks +2 lines, -42 lines 0 comments Download
M ash/shell_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +7 lines, -0 lines 0 comments Download
M ash/system/web_notification/ash_popup_alignment_delegate_unittest.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -28 lines 0 comments Download
D ash/wm/dock/docked_window_layout_manager_unittest.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -816 lines 0 comments Download
D ash/wm/dock/docked_window_resizer_unittest.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -1508 lines 0 comments Download
M ash/wm/maximize_mode/maximize_mode_window_manager_unittest.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -21 lines 0 comments Download
M ash/wm/overview/window_selector_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 5 chunks +0 lines, -305 lines 0 comments Download
M ash/wm/toplevel_window_event_handler_unittest.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +1 line, -32 lines 0 comments Download
M ash/wm/workspace/workspace_window_resizer_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +2 lines, -16 lines 0 comments Download
M ash/wm/workspace_controller_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 5 chunks +2 lines, -108 lines 0 comments Download
M chrome/app/generated_resources.grd View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +0 lines, -5 lines 0 comments Download
M chrome/browser/extensions/api/sessions/sessions_api.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/media/webrtc/desktop_media_list_ash.cc View 1 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/apps/chrome_native_app_window_views.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/ui/views/apps/chrome_native_app_window_views.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +4 lines, -14 lines 0 comments Download
M chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc View 2 chunks +0 lines, -10 lines 0 comments Download
M chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 4 chunks +2 lines, -22 lines 0 comments Download
M chrome/browser/ui/views/chrome_views_delegate.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_frame_ash.cc View 1 chunk +0 lines, -16 lines 0 comments Download
M chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +0 lines, -98 lines 0 comments Download
M chrome/browser/ui/views/tabs/tab_drag_controller.cc View 1 2 3 4 5 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +0 lines, -87 lines 0 comments Download
M chrome/browser/ui/window_sizer/window_sizer.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -11 lines 0 comments Download
M chrome/common/extensions/api/windows.json View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +16 lines, -1 line 0 comments Download
M components/sessions/core/session_service_commands.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 4 chunks +6 lines, -11 lines 0 comments Download
M services/ui/public/interfaces/window_manager_constants.mojom View 1 2 3 4 5 6 7 8 9 1 chunk +0 lines, -1 line 0 comments Download
M ui/aura/mus/property_converter.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -2 lines 0 comments Download
M ui/base/ui_base_types.h View 1 chunk +6 lines, -7 lines 0 comments Download
M ui/views/mus/desktop_window_tree_host_mus.h View 1 chunk +0 lines, -2 lines 0 comments Download
M ui/views/mus/desktop_window_tree_host_mus.cc View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +3 lines, -25 lines 0 comments Download
M ui/views/widget/native_widget_aura.h View 1 chunk +0 lines, -1 line 0 comments Download
M ui/views/widget/native_widget_aura.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +3 lines, -26 lines 0 comments Download
M ui/views/widget/native_widget_mac.mm View 1 2 1 chunk +0 lines, -1 line 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 108 (60 generated)
afakhry
Oshima-san, could you please review this CL? Thanks! I have currently two failing tests in ...
8 months ago (2017-02-22 02:40:48 UTC) #12
oshima
+varkha@ https://codereview.chromium.org/2700523004/diff/20001/ash/common/accelerators/accelerator_table.cc File ash/common/accelerators/accelerator_table.cc (right): https://codereview.chromium.org/2700523004/diff/20001/ash/common/accelerators/accelerator_table.cc#newcode461 ash/common/accelerators/accelerator_table.cc:461: TOGGLE_FULLSCREEN, TOGGLE_MAXIMIZED, WINDOW_POSITION_CENTER, I think we still want ...
8 months ago (2017-02-22 06:49:07 UTC) #16
oshima
and you need to update native_widget_mac as well. FAILED: obj/ui/views/views/native_widget_mac.o ../../ui/views/widget/native_widget_mac.mm:423:14: error: no member named ...
8 months ago (2017-02-22 06:50:01 UTC) #17
afakhry
https://codereview.chromium.org/2700523004/diff/20001/ash/common/accelerators/accelerator_table.cc File ash/common/accelerators/accelerator_table.cc (right): https://codereview.chromium.org/2700523004/diff/20001/ash/common/accelerators/accelerator_table.cc#newcode461 ash/common/accelerators/accelerator_table.cc:461: TOGGLE_FULLSCREEN, TOGGLE_MAXIMIZED, WINDOW_POSITION_CENTER, On 2017/02/22 06:49:07, oshima wrote: > ...
8 months ago (2017-02-22 22:04:44 UTC) #30
oshima
https://codereview.chromium.org/2700523004/diff/80001/ash/mus/accelerators/accelerator_controller_unittest.cc File ash/mus/accelerators/accelerator_controller_unittest.cc (right): https://codereview.chromium.org/2700523004/diff/80001/ash/mus/accelerators/accelerator_controller_unittest.cc#newcode530 ash/mus/accelerators/accelerator_controller_unittest.cc:530: // . http://crbug.com/615033: EventGenerator doesn't work with ash. That ...
7 months, 4 weeks ago (2017-02-23 02:21:03 UTC) #31
afakhry
https://codereview.chromium.org/2700523004/diff/80001/ash/mus/accelerators/accelerator_controller_unittest.cc File ash/mus/accelerators/accelerator_controller_unittest.cc (right): https://codereview.chromium.org/2700523004/diff/80001/ash/mus/accelerators/accelerator_controller_unittest.cc#newcode530 ash/mus/accelerators/accelerator_controller_unittest.cc:530: // . http://crbug.com/615033: EventGenerator doesn't work with ash. That ...
7 months, 4 weeks ago (2017-02-23 18:08:54 UTC) #34
oshima
https://codereview.chromium.org/2700523004/diff/140001/ash/wm/toplevel_window_event_handler_unittest.cc File ash/wm/toplevel_window_event_handler_unittest.cc (left): https://codereview.chromium.org/2700523004/diff/140001/ash/wm/toplevel_window_event_handler_unittest.cc#oldcode181 ash/wm/toplevel_window_event_handler_unittest.cc:181: client->SetCapture(nullptr); I believe this (releasing the capture) was a ...
7 months, 4 weeks ago (2017-02-23 22:40:02 UTC) #37
afakhry
https://codereview.chromium.org/2700523004/diff/140001/ash/wm/toplevel_window_event_handler_unittest.cc File ash/wm/toplevel_window_event_handler_unittest.cc (left): https://codereview.chromium.org/2700523004/diff/140001/ash/wm/toplevel_window_event_handler_unittest.cc#oldcode181 ash/wm/toplevel_window_event_handler_unittest.cc:181: client->SetCapture(nullptr); On 2017/02/23 22:40:02, oshima wrote: > I believe ...
7 months, 4 weeks ago (2017-02-24 20:00:55 UTC) #38
mfomitchev
https://codereview.chromium.org/2700523004/diff/160001/ash/public/cpp/shell_window_ids.h File ash/public/cpp/shell_window_ids.h (right): https://codereview.chromium.org/2700523004/diff/160001/ash/public/cpp/shell_window_ids.h#newcode89 ash/public/cpp/shell_window_ids.h:89: const int32_t kShellWindowId_DragImageAndTooltipContainer = 10; This should be 19, ...
7 months, 3 weeks ago (2017-02-25 03:35:46 UTC) #39
oshima
https://codereview.chromium.org/2700523004/diff/140001/ash/wm/toplevel_window_event_handler_unittest.cc File ash/wm/toplevel_window_event_handler_unittest.cc (left): https://codereview.chromium.org/2700523004/diff/140001/ash/wm/toplevel_window_event_handler_unittest.cc#oldcode181 ash/wm/toplevel_window_event_handler_unittest.cc:181: client->SetCapture(nullptr); On 2017/02/24 20:00:55, afakhry wrote: > On 2017/02/23 ...
7 months, 3 weeks ago (2017-02-27 15:10:08 UTC) #40
afakhry
https://codereview.chromium.org/2700523004/diff/140001/ash/wm/toplevel_window_event_handler_unittest.cc File ash/wm/toplevel_window_event_handler_unittest.cc (left): https://codereview.chromium.org/2700523004/diff/140001/ash/wm/toplevel_window_event_handler_unittest.cc#oldcode181 ash/wm/toplevel_window_event_handler_unittest.cc:181: client->SetCapture(nullptr); On 2017/02/27 15:10:08, oshima wrote: > On 2017/02/24 ...
7 months, 3 weeks ago (2017-02-27 17:43:50 UTC) #42
varkha
Thanks for doing this! A few places where I think we can remove more and ...
7 months, 3 weeks ago (2017-02-28 00:59:03 UTC) #46
varkha
A few more places that you may be able to remove some code or reference ...
7 months, 3 weeks ago (2017-02-28 01:32:22 UTC) #47
varkha
https://codereview.chromium.org/2700523004/diff/160001/ash/public/cpp/shell_window_ids.h File ash/public/cpp/shell_window_ids.h (right): https://codereview.chromium.org/2700523004/diff/160001/ash/public/cpp/shell_window_ids.h#newcode89 ash/public/cpp/shell_window_ids.h:89: const int32_t kShellWindowId_DragImageAndTooltipContainer = 10; On 2017/02/27 15:10:08, oshima ...
7 months, 3 weeks ago (2017-02-28 17:20:32 UTC) #48
afakhry
https://codereview.chromium.org/2700523004/diff/160001/ash/public/cpp/shell_window_ids.h File ash/public/cpp/shell_window_ids.h (right): https://codereview.chromium.org/2700523004/diff/160001/ash/public/cpp/shell_window_ids.h#newcode89 ash/public/cpp/shell_window_ids.h:89: const int32_t kShellWindowId_DragImageAndTooltipContainer = 10; On 2017/02/28 17:20:32, varkha ...
7 months, 2 weeks ago (2017-03-09 22:28:27 UTC) #51
sadrul
drive-by comment https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_window_ids.h File ash/public/cpp/shell_window_ids.h (right): https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_window_ids.h#newcode113 ash/public/cpp/shell_window_ids.h:113: const int32_t kAllShellContainerIds[] = { With the ...
7 months, 2 weeks ago (2017-03-10 01:18:29 UTC) #55
afakhry
https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_window_ids.h File ash/public/cpp/shell_window_ids.h (right): https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_window_ids.h#newcode113 ash/public/cpp/shell_window_ids.h:113: const int32_t kAllShellContainerIds[] = { On 2017/03/10 01:18:28, sadrul ...
7 months, 1 week ago (2017-03-14 02:07:30 UTC) #56
oshima
On 2017/03/14 02:07:30, afakhry wrote: > https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_window_ids.h > File ash/public/cpp/shell_window_ids.h (right): > > https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_window_ids.h#newcode113 > ...
7 months, 1 week ago (2017-03-14 02:34:52 UTC) #57
afakhry
Hi all, Friendly ping. I need to get this out of the way soon. Thanks!
7 months, 1 week ago (2017-03-15 03:06:10 UTC) #58
varkha
lgtm with a couple nits. Thanks again for doing this! https://codereview.chromium.org/2700523004/diff/240001/ash/accelerators/accelerator_controller_unittest.cc File ash/accelerators/accelerator_controller_unittest.cc (right): https://codereview.chromium.org/2700523004/diff/240001/ash/accelerators/accelerator_controller_unittest.cc#newcode492 ...
7 months, 1 week ago (2017-03-15 19:51:16 UTC) #59
afakhry
+sky@ for OWNERS review. https://codereview.chromium.org/2700523004/diff/240001/ash/accelerators/accelerator_controller_unittest.cc File ash/accelerators/accelerator_controller_unittest.cc (right): https://codereview.chromium.org/2700523004/diff/240001/ash/accelerators/accelerator_controller_unittest.cc#newcode492 ash/accelerators/accelerator_controller_unittest.cc:492: TEST_F(AcceleratorControllerTest, WindowSnapWithoutDocking) { On 2017/03/15 ...
7 months, 1 week ago (2017-03-15 20:27:26 UTC) #63
sky
On 2017/03/15 20:27:26, afakhry wrote: > +sky@ for OWNERS review. This is a big patch! ...
7 months, 1 week ago (2017-03-15 23:12:53 UTC) #66
afakhry
On 2017/03/15 23:12:53, sky wrote: > On 2017/03/15 20:27:26, afakhry wrote: > > +sky@ for ...
7 months, 1 week ago (2017-03-15 23:25:53 UTC) #67
oshima
my bits lgtm
7 months, 1 week ago (2017-03-15 23:25:57 UTC) #68
sky
Pleaes also make sure you remove stale includes. For example, I suspect there are a ...
7 months, 1 week ago (2017-03-16 04:24:24 UTC) #69
varkha
https://codereview.chromium.org/2700523004/diff/260001/chrome/common/extensions/api/windows.json File chrome/common/extensions/api/windows.json (left): https://codereview.chromium.org/2700523004/diff/260001/chrome/common/extensions/api/windows.json#oldcode23 chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", "docked"] On 2017/03/16 04:24:23, ...
7 months, 1 week ago (2017-03-16 14:13:54 UTC) #70
afakhry
Removed many unnecessary includes https://codereview.chromium.org/2700523004/diff/260001/chrome/common/extensions/api/windows.json File chrome/common/extensions/api/windows.json (left): https://codereview.chromium.org/2700523004/diff/260001/chrome/common/extensions/api/windows.json#oldcode23 chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", ...
7 months, 1 week ago (2017-03-16 17:50:24 UTC) #74
sky
LGTM, but make sure you want for feedback on the json change.
7 months, 1 week ago (2017-03-16 20:26:08 UTC) #77
afakhry
Thanks, sky! +rdevlin.cronin@ for chrome/common/extensions/api/windows.json. Devlin, please take a look, thank you!
7 months, 1 week ago (2017-03-16 22:50:39 UTC) #79
afakhry
+dcheng@ for services/ui/public/interfaces/window_manager_constants.mojom. Thank you!
7 months, 1 week ago (2017-03-16 22:53:07 UTC) #81
Devlin
https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json File chrome/common/extensions/api/windows.json (left): https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json#oldcode23 chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", "docked"] I don't see ...
7 months, 1 week ago (2017-03-17 02:24:40 UTC) #82
varkha
https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json File chrome/common/extensions/api/windows.json (left): https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json#oldcode23 chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", "docked"] On 2017/03/17 02:24:39, ...
7 months, 1 week ago (2017-03-17 03:39:44 UTC) #83
dcheng
rs lgtm for mojom change (but please make sure devlin's concerns are addressed before landing)
7 months, 1 week ago (2017-03-17 05:29:20 UTC) #84
Mike West
On 2017/03/16 at 17:50:24, afakhry wrote: > https://codereview.chromium.org/2700523004/diff/260001/chrome/common/extensions/api/windows.json#oldcode23 > chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", ...
7 months, 1 week ago (2017-03-17 12:33:27 UTC) #86
Devlin
https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json File chrome/common/extensions/api/windows.json (left): https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json#oldcode23 chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", "docked"] On 2017/03/17 03:39:44, ...
7 months, 1 week ago (2017-03-17 15:06:27 UTC) #87
afakhry
https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json File chrome/common/extensions/api/windows.json (left): https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json#oldcode23 chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", "docked"] On 2017/03/17 15:06:27, ...
7 months, 1 week ago (2017-03-17 17:43:23 UTC) #88
varkha
> - Do we also need to revert the changes in files: > window_manager_constants.mojom, > ...
7 months, 1 week ago (2017-03-17 17:52:47 UTC) #89
sky
window_manager_constants.mojom is entirely internal to chrome, so you can update it. -Scott On Fri, Mar ...
7 months, 1 week ago (2017-03-17 19:16:32 UTC) #90
Devlin
https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json File chrome/common/extensions/api/windows.json (left): https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json#oldcode23 chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", "docked"] On 2017/03/17 17:43:23, ...
7 months, 1 week ago (2017-03-17 19:27:00 UTC) #91
afakhry
Devlin, PTAL. Thanks! https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json File chrome/common/extensions/api/windows.json (left): https://codereview.chromium.org/2700523004/diff/280001/chrome/common/extensions/api/windows.json#oldcode23 chrome/common/extensions/api/windows.json:23: "enum": ["normal", "minimized", "maximized", "fullscreen", "docked"] ...
7 months ago (2017-03-17 23:34:59 UTC) #93
afakhry
Devlin, friendly ping. Thanks!
7 months ago (2017-03-21 02:08:57 UTC) #97
Devlin
afakhry wrote: > For the UMA part, I prefer to do this in a later ...
7 months ago (2017-03-21 17:34:52 UTC) #98
afakhry
On 2017/03/21 17:34:52, Devlin wrote: > afakhry wrote: > > For the UMA part, I ...
7 months ago (2017-03-21 18:09:12 UTC) #99
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2700523004/330001
7 months ago (2017-03-21 18:15:51 UTC) #102
commit-bot: I haz the power
Committed patchset #18 (id:330001) as https://chromium.googlesource.com/chromium/src/+/cd7193637823047ec88226019bb1d987215a35db
7 months ago (2017-03-21 19:29:01 UTC) #105
sadrul
On 2017/03/10 01:18:29, sadrul wrote: > drive-by comment > > https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_window_ids.h > File ash/public/cpp/shell_window_ids.h (right): ...
6 months, 1 week ago (2017-04-13 17:00:21 UTC) #106
afakhry
On 2017/04/13 17:00:21, sadrul wrote: > On 2017/03/10 01:18:29, sadrul wrote: > > drive-by comment ...
6 months, 1 week ago (2017-04-13 19:55:55 UTC) #107
afakhry
6 months, 1 week ago (2017-04-13 22:53:07 UTC) #108
Message was sent while issue was closed.
On 2017/04/13 19:55:55, afakhry wrote:
> On 2017/04/13 17:00:21, sadrul wrote:
> > On 2017/03/10 01:18:29, sadrul wrote:
> > > drive-by comment
> > > 
> > >
> >
>
https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_w...
> > > File ash/public/cpp/shell_window_ids.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2700523004/diff/220001/ash/public/cpp/shell_w...
> > > ash/public/cpp/shell_window_ids.h:113: const int32_t
kAllShellContainerIds[]
> =
> > {
> > > With the introduction of enums, this will go away, right? Instead of
adding
> > this
> > > with this CL, and removing it later, would it be possible to convert to
> enums
> > > first, and then land this CL? (that would also make it easier to do a
revert
> > if
> > > needed for some reason)
> > 
> > The CL has landed a while back. Can you please fix this now?
> 
> I have a CL for this: https://codereview.chromium.org/2751523003/ but I need
to
> rebase it. I will do this very soon. Thanks for the reminder!

Done. Doing a dry-run to make sure everything is ok, then I'll have it reviewed.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa