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

Issue 2500973002: Converts test_wm to use aura (Closed)

Created:
4 years, 1 month ago by sky
Modified:
4 years ago
Reviewers:
msw, bruthig, sadrul, Fady Samuel
CC:
chromium-reviews, rjkroege, kalyank, bruthig
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Converts test_wm to use aura BUG=664625 TEST=covered by tests R=msw@chromium.org Committed: https://crrev.com/bec8c81f5fa63932e2c95d91d800b7486f40ab0d Cr-Commit-Position: refs/heads/master@{#434203}

Patch Set 1 #

Patch Set 2 : cleanup #

Total comments: 6

Patch Set 3 : Feedback and fix from Sadrul #

Patch Set 4 : merge 2 trunk #

Patch Set 5 : fix bad remove #

Total comments: 8

Patch Set 6 : delete cant be used #

Total comments: 1

Patch Set 7 : feedback #

Total comments: 2

Patch Set 8 : oopsie #

Total comments: 3

Patch Set 9 : ! #

Patch Set 10 : merge #

Patch Set 11 : update args #

Patch Set 12 : fix #

Patch Set 13 : merge #

Patch Set 14 : tweak #

Total comments: 1

Patch Set 15 : reset root_ and disable #

Patch Set 16 : nuke gl #

Patch Set 17 : no new args #

Total comments: 5

Patch Set 18 : reference bug #

Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -30 lines) Patch
M services/ui/service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +2 lines, -0 lines 0 comments Download
M services/ui/test_wm/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download
M services/ui/test_wm/manifest.json View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M services/ui/test_wm/test_wm.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 3 chunks +84 lines, -28 lines 0 comments Download
M ui/views/animation/ink_drop_impl_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +6 lines, -0 lines 0 comments Download
M ui/views/mus/input_method_mus_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -1 line 0 comments Download
M ui/wm/core/wm_state.h View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 71 (37 generated)
sky
4 years, 1 month ago (2016-11-14 16:56:15 UTC) #1
msw
lgtm with nits and a q. https://codereview.chromium.org/2500973002/diff/20001/services/ui/test_wm/test_wm.cc File services/ui/test_wm/test_wm.cc (right): https://codereview.chromium.org/2500973002/diff/20001/services/ui/test_wm/test_wm.cc#newcode54 services/ui/test_wm/test_wm.cc:54: ui::InitializeContextFactoryForTests(enable_pixel_output)); q: Should ...
4 years, 1 month ago (2016-11-14 19:23:10 UTC) #2
sky
+sadrul I this is finally ready for re-review. I had to add some code from ...
4 years, 1 month ago (2016-11-17 20:29:34 UTC) #6
msw
still lgtm with nits and minor comments. https://codereview.chromium.org/2500973002/diff/80001/services/ui/test_wm/test_wm.cc File services/ui/test_wm/test_wm.cc (right): https://codereview.chromium.org/2500973002/diff/80001/services/ui/test_wm/test_wm.cc#newcode50 services/ui/test_wm/test_wm.cc:50: ui::TerminateContextFactoryForTests(); Is ...
4 years, 1 month ago (2016-11-17 21:26:15 UTC) #9
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/2500973002/120001
4 years, 1 month ago (2016-11-17 22:24:22 UTC) #12
sky
https://codereview.chromium.org/2500973002/diff/80001/services/ui/test_wm/test_wm.cc File services/ui/test_wm/test_wm.cc (right): https://codereview.chromium.org/2500973002/diff/80001/services/ui/test_wm/test_wm.cc#newcode50 services/ui/test_wm/test_wm.cc:50: ui::TerminateContextFactoryForTests(); On 2016/11/17 21:26:14, msw wrote: > Is this ...
4 years, 1 month ago (2016-11-17 22:25:55 UTC) #13
msw
slgtm with a q https://codereview.chromium.org/2500973002/diff/120001/services/ui/ws/window_tree.cc File services/ui/ws/window_tree.cc (right): https://codereview.chromium.org/2500973002/diff/120001/services/ui/ws/window_tree.cc#newcode311 services/ui/ws/window_tree.cc:311: << " local window_id= " ...
4 years, 1 month ago (2016-11-17 22:27:27 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: blimp_linux_dbg on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/blimp_linux_dbg/builds/34984) chromeos_x86-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
4 years, 1 month ago (2016-11-17 23:36:04 UTC) #16
sky
https://codereview.chromium.org/2500973002/diff/120001/services/ui/ws/window_tree.cc File services/ui/ws/window_tree.cc (right): https://codereview.chromium.org/2500973002/diff/120001/services/ui/ws/window_tree.cc#newcode311 services/ui/ws/window_tree.cc:311: << " local window_id= " << window_id.id << " ...
4 years, 1 month ago (2016-11-18 00:56:24 UTC) #17
sadrul
lgtm https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc File services/ui/test_wm/test_wm.cc (right): https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc#newcode40 services/ui/test_wm/test_wm.cc:40: TestWM() { gl::GLSurfaceTestSupport::InitializeOneOff(); } Do you need to ...
4 years, 1 month ago (2016-11-18 01:03:53 UTC) #18
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/2500973002/160001
4 years, 1 month ago (2016-11-18 01:08:37 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/109361) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, ...
4 years, 1 month ago (2016-11-18 01:14:21 UTC) #23
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/2500973002/180001
4 years, 1 month ago (2016-11-18 02:41:28 UTC) #26
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/317755)
4 years, 1 month ago (2016-11-18 04:30:34 UTC) #28
sky
https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc File services/ui/test_wm/test_wm.cc (right): https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc#newcode40 services/ui/test_wm/test_wm.cc:40: TestWM() { gl::GLSurfaceTestSupport::InitializeOneOff(); } On 2016/11/18 01:03:53, sadrul wrote: ...
4 years, 1 month ago (2016-11-18 05:11:06 UTC) #31
sky
On 2016/11/18 05:11:06, sky wrote: > https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc > File services/ui/test_wm/test_wm.cc (right): > > https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc#newcode40 > ...
4 years, 1 month ago (2016-11-18 05:11:23 UTC) #32
sky
I believe I've resoled all the issues blocking this and am going to try to ...
4 years, 1 month ago (2016-11-22 04:27:10 UTC) #39
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/2500973002/260001
4 years, 1 month ago (2016-11-22 04:27:41 UTC) #42
sadrul
On 2016/11/22 04:27:10, sky wrote: > I believe I've resoled all the issues blocking this ...
4 years, 1 month ago (2016-11-22 04:33:11 UTC) #43
sadrul
On 2016/11/22 04:33:11, sadrul wrote: > On 2016/11/22 04:27:10, sky wrote: > > I believe ...
4 years, 1 month ago (2016-11-22 04:34:19 UTC) #44
sky
Thanks! On Mon, Nov 21, 2016 at 8:34 PM, <sadrul@chromium.org> wrote: > On 2016/11/22 04:33:11, ...
4 years, 1 month ago (2016-11-22 04:37:54 UTC) #45
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/342382)
4 years, 1 month ago (2016-11-22 07:06:10 UTC) #47
bruthig
Just adding myself to CC to watch for when this lands.
4 years, 1 month ago (2016-11-22 15:31:30 UTC) #49
msw
still lgtm with a q https://codereview.chromium.org/2500973002/diff/260001/services/ui/test_wm/test_wm.cc File services/ui/test_wm/test_wm.cc (right): https://codereview.chromium.org/2500973002/diff/260001/services/ui/test_wm/test_wm.cc#newcode146 services/ui/test_wm/test_wm.cc:146: void OnWmDisplayRemoved(aura::WindowTreeHostMus* window_tree_host) override ...
4 years, 1 month ago (2016-11-22 18:13:57 UTC) #50
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/2500973002/260001
4 years, 1 month ago (2016-11-22 20:14:10 UTC) #52
Fady Samuel
https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc File services/ui/test_wm/test_wm.cc (right): https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc#newcode40 services/ui/test_wm/test_wm.cc:40: TestWM() { gl::GLSurfaceTestSupport::InitializeOneOff(); } On 2016/11/18 05:11:06, sky wrote: ...
4 years, 1 month ago (2016-11-23 00:39:54 UTC) #58
sky
On 2016/11/23 00:39:54, Fady Samuel wrote: > https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc > File services/ui/test_wm/test_wm.cc (right): > > https://codereview.chromium.org/2500973002/diff/140001/services/ui/test_wm/test_wm.cc#newcode40 ...
4 years, 1 month ago (2016-11-23 05:59:08 UTC) #59
sadrul
lgtm https://codereview.chromium.org/2500973002/diff/320001/services/ui/service.cc File services/ui/service.cc (right): https://codereview.chromium.org/2500973002/diff/320001/services/ui/service.cc#newcode153 services/ui/service.cc:153: ui::SetDefaultX11ErrorHandlers(); I don't think this has an effect ...
4 years, 1 month ago (2016-11-23 06:19:53 UTC) #60
sky
https://codereview.chromium.org/2500973002/diff/320001/services/ui/service.cc File services/ui/service.cc (right): https://codereview.chromium.org/2500973002/diff/320001/services/ui/service.cc#newcode153 services/ui/service.cc:153: ui::SetDefaultX11ErrorHandlers(); On 2016/11/23 06:19:53, sadrul wrote: > I don't ...
4 years ago (2016-11-23 16:22:11 UTC) #62
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/2500973002/340001
4 years ago (2016-11-23 16:22:41 UTC) #65
sadrul
https://codereview.chromium.org/2500973002/diff/320001/services/ui/service.cc File services/ui/service.cc (right): https://codereview.chromium.org/2500973002/diff/320001/services/ui/service.cc#newcode153 services/ui/service.cc:153: ui::SetDefaultX11ErrorHandlers(); On 2016/11/23 16:22:11, sky wrote: > On 2016/11/23 ...
4 years ago (2016-11-23 16:27:38 UTC) #66
sky
We run the ui service in the x11 non-ozone config. Maybe we shouldn't? On Wed, ...
4 years ago (2016-11-23 16:31:49 UTC) #67
commit-bot: I haz the power
Committed patchset #18 (id:340001)
4 years ago (2016-11-23 18:26:24 UTC) #69
commit-bot: I haz the power
4 years ago (2016-11-23 18:31:41 UTC) #71
Message was sent while issue was closed.
Patchset 18 (id:??) landed as
https://crrev.com/bec8c81f5fa63932e2c95d91d800b7486f40ab0d
Cr-Commit-Position: refs/heads/master@{#434203}

Powered by Google App Engine
This is Rietveld 408576698