Chromium Code Reviews
Help | Chromium Project | Sign in
(1)

Issue 2733313002: Move display preference code from ash/display/ to ui/display/manager/. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks, 2 days ago by thanhph
Modified:
6 days, 3 hours ago
CC:
chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move display preference code from ash/display/ to ui/display/manager/. This cl moves Ash dependencies out of display_preferences.cc. This is an effort to separate Ash and Chrome in the new split processes architecture and make DisplayPreferences work with mustash. BUG=678949 Review-Url: https://codereview.chromium.org/2733313002 Cr-Commit-Position: refs/heads/master@{#457974} Committed: https://chromium.googlesource.com/chromium/src/+/9a5d265ee2cc632d0c6c6ee7bcef12c2482d5bf4

Patch Set 1 : . #

Total comments: 6

Patch Set 2 : . #

Total comments: 2

Patch Set 3 : Add back new lines. #

Patch Set 4 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+78 lines, -445 lines) Patch
M ash/BUILD.gn View 1 2 3 2 chunks +0 lines, -4 lines 0 comments Download
D ash/display/display_pref_util.h View 1 chunk +0 lines, -64 lines 0 comments Download
D ash/display/json_converter.h View 1 chunk +0 lines, -28 lines 0 comments Download
D ash/display/json_converter.cc View 1 chunk +0 lines, -201 lines 0 comments Download
D ash/display/json_converter_unittest.cc View 1 chunk +0 lines, -95 lines 0 comments Download
M chrome/browser/chromeos/BUILD.gn View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/display/display_preferences.cc View 6 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/display/display_preferences_unittest.cc View 11 chunks +11 lines, -11 lines 0 comments Download
M ui/display/BUILD.gn View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M ui/display/manager/BUILD.gn View 1 1 chunk +3 lines, -0 lines 0 comments Download
A + ui/display/manager/display_pref_util.h View 2 chunks +5 lines, -5 lines 0 comments Download
A ui/display/manager/json_converter.h View 1 2 1 chunk +25 lines, -0 lines 0 comments Download
A + ui/display/manager/json_converter.cc View 1 9 chunks +15 lines, -19 lines 0 comments Download
A + ui/display/manager/json_converter_unittest.cc View 1 4 chunks +11 lines, -12 lines 0 comments Download
Commit queue not available (can’t edit this change).

Dependent Patchsets:

Messages

Total messages: 56 (37 generated)
thanhph
Hi Kyle, Can you review my cl? Thanks, Thanh.
2 weeks, 2 days ago (2017-03-08 14:20:57 UTC) #11
kylechar
Can you update the CL description to say why you want to move the code? ...
2 weeks, 2 days ago (2017-03-08 15:28:15 UTC) #13
thanhph
Thanks Kyle. Can you review my new patch? Thanks, Thanh. https://codereview.chromium.org/2733313002/diff/40001/chrome/browser/chromeos/BUILD.gn File chrome/browser/chromeos/BUILD.gn (right): https://codereview.chromium.org/2733313002/diff/40001/chrome/browser/chromeos/BUILD.gn#newcode33 ...
2 weeks, 1 day ago (2017-03-09 13:48:29 UTC) #24
kylechar
https://codereview.chromium.org/2733313002/diff/80001/ui/display/manager/json_converter.h File ui/display/manager/json_converter.h (right): https://codereview.chromium.org/2733313002/diff/80001/ui/display/manager/json_converter.h#newcode15 ui/display/manager/json_converter.h:15: class DisplayLayout; What happened to all the new lines ...
2 weeks, 1 day ago (2017-03-09 13:59:25 UTC) #25
thanhph
https://codereview.chromium.org/2733313002/diff/80001/ui/display/manager/json_converter.h File ui/display/manager/json_converter.h (right): https://codereview.chromium.org/2733313002/diff/80001/ui/display/manager/json_converter.h#newcode15 ui/display/manager/json_converter.h:15: class DisplayLayout; On 2017/03/09 13:59:25, kylechar wrote: > What ...
2 weeks, 1 day ago (2017-03-09 14:16:08 UTC) #26
kylechar
lgtm
2 weeks, 1 day ago (2017-03-09 14:16:36 UTC) #27
thanhph
Thanks Kyle. Rob: Can you review my CL? Thanks, Thanh.
2 weeks, 1 day ago (2017-03-09 14:19:35 UTC) #29
thanhph
Hi Daniel, Could you review my CL? Thanks a lot, Thanh.
1 week ago (2017-03-17 13:44:27 UTC) #35
Daniel Erat
lgtm
1 week ago (2017-03-17 14:40:12 UTC) #36
rjkroege
lgtm.
1 week ago (2017-03-17 14:43:16 UTC) #37
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/2733313002/100001
1 week ago (2017-03-17 14:52:03 UTC) #39
thanhph
On 2017/03/17 14:43:16, rjkroege wrote: > lgtm. Thanks Rob and Daniel. I'll try the commit ...
1 week ago (2017-03-17 14:52:14 UTC) #40
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/252534)
1 week ago (2017-03-17 16:19:02 UTC) #42
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/2733313002/100001
1 week ago (2017-03-17 17:19:27 UTC) #44
commit-bot: I haz the power
Failed to apply patch for chrome/browser/chromeos/BUILD.gn: While running git apply --index -p1; error: patch failed: ...
1 week ago (2017-03-17 17:59:29 UTC) #46
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/2733313002/120001
1 week ago (2017-03-17 21:12:44 UTC) #49
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/386674)
6 days, 22 hours ago (2017-03-17 23:08:50 UTC) #51
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/2733313002/120001
6 days, 4 hours ago (2017-03-18 17:53:36 UTC) #53
commit-bot: I haz the power
6 days, 3 hours ago (2017-03-18 18:57:57 UTC) #56
Message was sent while issue was closed.
Committed patchset #4 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/9a5d265ee2cc632d0c6c6ee7bcef...
Sign in to reply to this message.

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