Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2361283002:
Add GetDisplayWithDisplayId to display::Screen. (Closed)
|
Created:
4 years, 3 months ago by riajiang Modified:
4 years, 1 month ago CC:
chromium-reviews, extensions-reviews_chromium.org, anandc+watch-blimp_chromium.org, sadrul, sriramsr+watch-blimp_chromium.org, tfarina, maniscalco+watch-blimp_chromium.org, steimel+watch-blimp_chromium.org, gcasto+watch-blimp_chromium.org, shaktisahu+watch-blimp_chromium.org, nyquist+watch-blimp_chromium.org, perumaal+watch-blimp_chromium.org, marcinjb+watch-blimp_chromium.org, jessicag+watch-blimp_chromium.org, kmarshall+watch-blimp_chromium.org, lethalantidote+watch-blimp_chromium.org, chromium-apps-reviews_chromium.org, kalyank, dtrainor+watch-blimp_chromium.org, scf+watch-blimp_chromium.org, khushalsagar+watch-blimp_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd GetDisplayWithDisplayId to display::Screen.
BUG=600815
TEST=ash_unittests blimp_unittests app_shell_unittests
display_unittests views_unittests
Committed: https://crrev.com/29f6a42748e827c3de474c76d6d2f56a78369d9c
Cr-Commit-Position: refs/heads/master@{#433618}
Patch Set 1 #Patch Set 2 : test #
Total comments: 12
Patch Set 3 : ref, test, etc #
Total comments: 6
Patch Set 4 : id #
Total comments: 5
Patch Set 5 : display_list #Patch Set 6 : make GetDisplayWithDisplayId non virtual #Patch Set 7 : rebase #
Total comments: 3
Patch Set 8 : move unittest #
Messages
Total messages: 60 (42 generated)
The CQ bit was checked by riajiang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by riajiang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Add GetDisplayWithDisplayId to display::Screen. BUG= ========== to ========== Add GetDisplayWithDisplayId to display::Screen. BUG=600815 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
The CQ bit was checked by riajiang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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/bui...)
The CQ bit was checked by riajiang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #4 (id:60001) has been deleted
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by riajiang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #3 (id:40001) has been deleted
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #2 (id:100001) has been deleted
riajiang@chromium.org changed reviewers: + sadrul@chromium.org
sadrul@chromium.org changed reviewers: + sky@chromium.org
+sky@ as well. I have left a few comments in a few places. The main thing to note: avoid copies we don't need. The GetAllDisplays() does a copy of the std::vector<>, and we should avoid that. I think this is a useful function to have on display::Screen. We can update some code (e.g. NativeWidgetMus::GetWindowBoundsInScreen(), GetCenterOfDisplay() in app_list_presenter_delegate_mus.cc) to use this instead of iterating over the list of displays. However, looking at this patch, I feel like we should also consolidate the various Screen impls. For example, BlimpScreen, CastScreen, extensions::ShellScreen, aura::test::TestScreen could all become a SingleDisplayScreen (or something like that). https://codereview.chromium.org/2361283002/diff/120001/ash/display/screen_ash.cc File ash/display/screen_ash.cc (right): https://codereview.chromium.org/2361283002/diff/120001/ash/display/screen_ash... ash/display/screen_ash.cc:53: for (display::Display display_in_list : display_list_) { const& https://codereview.chromium.org/2361283002/diff/120001/ash/display/screen_ash... ash/display/screen_ash.cc:129: for (display::Display display_in_list : GetAllDisplays()) { GetAllDisplays() makes a copy of the list. It'd be better to iterate over the list without having to create a copy of it first. So this should be like: for (const Display& d : GetDisplayManager()->active_display_list()) { .... } https://codereview.chromium.org/2361283002/diff/120001/blimp/engine/app/ui/bl... File blimp/engine/app/ui/blimp_screen_unittest.cc (right): https://codereview.chromium.org/2361283002/diff/120001/blimp/engine/app/ui/bl... blimp/engine/app/ui/blimp_screen_unittest.cc:118: EXPECT_FALSE(has_display2); Should be TRUE? https://codereview.chromium.org/2361283002/diff/120001/chrome/browser/ui/wind... File chrome/browser/ui/window_sizer/window_sizer_common_unittest.cc (right): https://codereview.chromium.org/2361283002/diff/120001/chrome/browser/ui/wind... chrome/browser/ui/window_sizer/window_sizer_common_unittest.cc:66: for (display::Display display_in_list : displays_) { const & https://codereview.chromium.org/2361283002/diff/120001/ui/display/screen.h File ui/display/screen.h (right): https://codereview.chromium.org/2361283002/diff/120001/ui/display/screen.h#ne... ui/display/screen.h:58: // Returns the display with |display_id|. Mention that if the display with the specified id is not found, then false is returned, and |display| remains untouched. https://codereview.chromium.org/2361283002/diff/120001/ui/display/screen_base.cc File ui/display/screen_base.cc (right): https://codereview.chromium.org/2361283002/diff/120001/ui/display/screen_base... ui/display/screen_base.cc:59: for (display::Display display_in_list : GetAllDisplays()) { Iterate over display_list_.displays() to avoid extra copy. Also, const&
I have left a few comments, but let's discuss offline before addressing these. https://codereview.chromium.org/2361283002/diff/140001/ui/display/android/scr... File ui/display/android/screen_android.cc (right): https://codereview.chromium.org/2361283002/diff/140001/ui/display/android/scr... ui/display/android/screen_android.cc:76: } You can probably just do: if (display_id == 0) { *display = GetPrimaryDisplay(); return true; } return false; Considering the display created always has id 0 (line 46) https://codereview.chromium.org/2361283002/diff/140001/ui/display/ios/screen_... File ui/display/ios/screen_ios.mm (right): https://codereview.chromium.org/2361283002/diff/140001/ui/display/ios/screen_... ui/display/ios/screen_ios.mm:48: NOTIMPLEMENTED(); Return the primary display if display_id == 0 https://codereview.chromium.org/2361283002/diff/140001/ui/display/win/screen_... File ui/display/win/screen_win.cc (right): https://codereview.chromium.org/2361283002/diff/140001/ui/display/win/screen_... ui/display/win/screen_win.cc:394: ScreenWinDisplaysToDisplays(screen_win_displays_)) { This too is creating a new list that we don't need. This should look like: for (const auto& disp : screen_win_displays_) { if (disp.display().id() == display_id) { *display = disp.display(); return true; } } return false;
Decided to separate the work on consolidating subclasses of display::Screen that only have one display::Display into another CL. Could you take a look at this part first? Thanks! https://codereview.chromium.org/2361283002/diff/120001/ash/display/screen_ash.cc File ash/display/screen_ash.cc (right): https://codereview.chromium.org/2361283002/diff/120001/ash/display/screen_ash... ash/display/screen_ash.cc:53: for (display::Display display_in_list : display_list_) { On 2016/09/23 19:51:12, sadrul wrote: > const& Done. https://codereview.chromium.org/2361283002/diff/120001/ash/display/screen_ash... ash/display/screen_ash.cc:129: for (display::Display display_in_list : GetAllDisplays()) { On 2016/09/23 19:51:12, sadrul wrote: > GetAllDisplays() makes a copy of the list. It'd be better to iterate over the > list without having to create a copy of it first. So this should be like: > > for (const Display& d : GetDisplayManager()->active_display_list()) { > .... > } I see. Done. https://codereview.chromium.org/2361283002/diff/120001/blimp/engine/app/ui/bl... File blimp/engine/app/ui/blimp_screen_unittest.cc (right): https://codereview.chromium.org/2361283002/diff/120001/blimp/engine/app/ui/bl... blimp/engine/app/ui/blimp_screen_unittest.cc:118: EXPECT_FALSE(has_display2); On 2016/09/23 19:51:12, sadrul wrote: > Should be TRUE? Yes! https://codereview.chromium.org/2361283002/diff/120001/chrome/browser/ui/wind... File chrome/browser/ui/window_sizer/window_sizer_common_unittest.cc (right): https://codereview.chromium.org/2361283002/diff/120001/chrome/browser/ui/wind... chrome/browser/ui/window_sizer/window_sizer_common_unittest.cc:66: for (display::Display display_in_list : displays_) { On 2016/09/23 19:51:12, sadrul wrote: > const & Done. https://codereview.chromium.org/2361283002/diff/120001/ui/display/screen.h File ui/display/screen.h (right): https://codereview.chromium.org/2361283002/diff/120001/ui/display/screen.h#ne... ui/display/screen.h:58: // Returns the display with |display_id|. On 2016/09/23 19:51:12, sadrul wrote: > Mention that if the display with the specified id is not found, then false is > returned, and |display| remains untouched. Done. https://codereview.chromium.org/2361283002/diff/120001/ui/display/screen_base.cc File ui/display/screen_base.cc (right): https://codereview.chromium.org/2361283002/diff/120001/ui/display/screen_base... ui/display/screen_base.cc:59: for (display::Display display_in_list : GetAllDisplays()) { On 2016/09/23 19:51:12, sadrul wrote: > Iterate over display_list_.displays() to avoid extra copy. Also, const& Done. https://codereview.chromium.org/2361283002/diff/140001/ui/display/android/scr... File ui/display/android/screen_android.cc (right): https://codereview.chromium.org/2361283002/diff/140001/ui/display/android/scr... ui/display/android/screen_android.cc:76: } On 2016/09/30 03:17:14, sadrul wrote: > You can probably just do: > if (display_id == 0) { > *display = GetPrimaryDisplay(); > return true; > } > return false; > > Considering the display created always has id 0 (line 46) Done. https://codereview.chromium.org/2361283002/diff/140001/ui/display/ios/screen_... File ui/display/ios/screen_ios.mm (right): https://codereview.chromium.org/2361283002/diff/140001/ui/display/ios/screen_... ui/display/ios/screen_ios.mm:48: NOTIMPLEMENTED(); On 2016/09/30 03:17:14, sadrul wrote: > Return the primary display if display_id == 0 Done. https://codereview.chromium.org/2361283002/diff/140001/ui/display/win/screen_... File ui/display/win/screen_win.cc (right): https://codereview.chromium.org/2361283002/diff/140001/ui/display/win/screen_... ui/display/win/screen_win.cc:394: ScreenWinDisplaysToDisplays(screen_win_displays_)) { On 2016/09/30 03:17:14, sadrul wrote: > This too is creating a new list that we don't need. This should look like: > > for (const auto& disp : screen_win_displays_) { > if (disp.display().id() == display_id) { > *display = disp.display(); > return true; > } > } > return false; Done.
Description was changed from ========== Add GetDisplayWithDisplayId to display::Screen. BUG=600815 ========== to ========== Add GetDisplayWithDisplayId to display::Screen. BUG=600815 TEST=ash_unittests blimp_unittests app_shell_unittests display_unittests views_unittests ==========
Description was changed from
==========
Add GetDisplayWithDisplayId to display::Screen.
BUG=600815
TEST=ash_unittests blimp_unittests app_shell_unittests
display_unittests views_unittests
==========
to
==========
Add GetDisplayWithDisplayId to display::Screen.
BUG=600815
TEST=ash_unittests blimp_unittests app_shell_unittests
display_unittests views_unittests
==========
https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h File ui/display/screen.h (right): https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h#ne... ui/display/screen.h:58: // Returns true if the display with |display_id| is found and returns that Is there a reason this can't be made a helper function and not a pure virtual function of this class? Doesn't iterating over GetAllDisplays() give you what you need?
You are working on using display::DisplayList in more places, right? That would be good to get rid of a lot of similar code in this CL. For now, this lgtm https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen_base.cc File ui/display/screen_base.cc (right): https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen_base... ui/display/screen_base.cc:59: for (const display::Display& display_in_list : display_list_.displays()) { Use DisplayList::FindDisplayById() here instead, so: auto iter = display_list_.FindDisplayById(display_id); if (iter == display_list_.displays().end()) return false; *display = *iter; return true;
https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h File ui/display/screen.h (right): https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h#ne... ui/display/screen.h:58: // Returns true if the display with |display_id| is found and returns that On 2016/10/06 15:20:03, sky wrote: > Is there a reason this can't be made a helper function and not a pure virtual > function of this class? Doesn't iterating over GetAllDisplays() give you what > you need? We were thinking to change some subclasses of display::Screen to use ScreenBase instead to make use of the DisplayList in ScreenBase and to avoid similar code in those subclasses. DisplayList already has a FindDisplayById(). And GetAllDisplays() would make a copy. What do you think? https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen_base.cc File ui/display/screen_base.cc (right): https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen_base... ui/display/screen_base.cc:59: for (const display::Display& display_in_list : display_list_.displays()) { On 2016/10/06 15:23:18, sadrul wrote: > Use DisplayList::FindDisplayById() here instead, so: > > auto iter = display_list_.FindDisplayById(display_id); > if (iter == display_list_.displays().end()) > return false; > *display = *iter; > return true; Done. Sorry, forgot this change about DisplayList in the ScreenBase branch...
https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h File ui/display/screen.h (right): https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h#ne... ui/display/screen.h:58: // Returns true if the display with |display_id| is found and returns that On 2016/10/06 18:26:31, riajiang wrote: > On 2016/10/06 15:20:03, sky wrote: > > Is there a reason this can't be made a helper function and not a pure virtual > > function of this class? Doesn't iterating over GetAllDisplays() give you what > > you need? > > We were thinking to change some subclasses of display::Screen to use ScreenBase > instead to make use of the DisplayList in ScreenBase and to avoid similar code > in those subclasses. That seems orthogonal. You could change subclasses to use DisplayList regardless of this change. > DisplayList already has a FindDisplayById(). And > GetAllDisplays() would make a copy. What do you think? I am more in favor of making GetAllDisplays() return a const vector& and move this function else where. That makes it easier for subclasses to implement Screen. WDYT?
On 2016/10/06 21:28:34, sky wrote: > https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h > File ui/display/screen.h (right): > > https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h#ne... > ui/display/screen.h:58: // Returns true if the display with |display_id| is > found and returns that > On 2016/10/06 18:26:31, riajiang wrote: > > On 2016/10/06 15:20:03, sky wrote: > > > Is there a reason this can't be made a helper function and not a pure > virtual > > > function of this class? Doesn't iterating over GetAllDisplays() give you > what > > > you need? > > > > We were thinking to change some subclasses of display::Screen to use > ScreenBase > > instead to make use of the DisplayList in ScreenBase and to avoid similar code > > in those subclasses. > > That seems orthogonal. You could change subclasses to use DisplayList regardless > of this change. > > > DisplayList already has a FindDisplayById(). And > > GetAllDisplays() would make a copy. What do you think? > > I am more in favor of making GetAllDisplays() return a const vector& and move > this function else where. That makes it easier for subclasses to implement > Screen. WDYT? Changed GetDisplayWithDisplayId to be a non-virtual function in display::Screen and use GetAllDisplays. Going to 1) change single display screens to use ScreenBase / DisplayList and 2) change GetAllDisplays to return a const vector& in another CL (right now this https://cs.chromium.org/chromium/src/blimp/engine/app/ui/blimp_screen.cc?cl=G... in single display screens makes returning const vector& not working)
On 2016/10/13 21:39:51, riajiang wrote: > On 2016/10/06 21:28:34, sky wrote: > > https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h > > File ui/display/screen.h (right): > > > > > https://codereview.chromium.org/2361283002/diff/160001/ui/display/screen.h#ne... > > ui/display/screen.h:58: // Returns true if the display with |display_id| is > > found and returns that > > On 2016/10/06 18:26:31, riajiang wrote: > > > On 2016/10/06 15:20:03, sky wrote: > > > > Is there a reason this can't be made a helper function and not a pure > > virtual > > > > function of this class? Doesn't iterating over GetAllDisplays() give you > > what > > > > you need? > > > > > > We were thinking to change some subclasses of display::Screen to use > > ScreenBase > > > instead to make use of the DisplayList in ScreenBase and to avoid similar > code > > > in those subclasses. > > > > That seems orthogonal. You could change subclasses to use DisplayList > regardless > > of this change. > > > > > DisplayList already has a FindDisplayById(). And > > > GetAllDisplays() would make a copy. What do you think? > > > > I am more in favor of making GetAllDisplays() return a const vector& and move > > this function else where. That makes it easier for subclasses to implement > > Screen. WDYT? > > Changed GetDisplayWithDisplayId to be a non-virtual function in display::Screen > and > use GetAllDisplays. Going to 1) change single display screens to use ScreenBase > / > DisplayList and 2) change GetAllDisplays to return a const vector& in another CL > (right now this > https://cs.chromium.org/chromium/src/blimp/engine/app/ui/blimp_screen.cc?cl=G... > in single display screens makes returning const vector& not working) If you make GetAllDisplays to return a const vector& return a const, then won't that mean you don't need GetDisplayWithDisplayId? I guess I'm asking why you can't do the const vector& change first?
riajiang@chromium.org changed reviewers: + dtrainor@chromium.org, jamescook@chromium.org
The CL about GetAllDisplays() (https://codereview.chromium.org/2489873002/) has been landed. sky@, could you take another look at ash? Also +dtrainor@ for blimp and +jamescook@ for extensions/shell. Thanks!
extensions/shell LGTM https://codereview.chromium.org/2361283002/diff/220001/extensions/shell/brows... File extensions/shell/browser/shell_screen_unittest.cc (right): https://codereview.chromium.org/2361283002/diff/220001/extensions/shell/brows... extensions/shell/browser/shell_screen_unittest.cc:31: EXPECT_TRUE(has_display); nit: EXPECT_TRUE(screen.GetDisplayWithDisplayId(0, &display));
https://codereview.chromium.org/2361283002/diff/220001/ui/display/win/screen_... File ui/display/win/screen_win_unittest.cc (right): https://codereview.chromium.org/2361283002/diff/220001/ui/display/win/screen_... ui/display/win/screen_win_unittest.cc:362: EXPECT_TRUE(has_display); no need for the temporary. This comment holds for all tests where I think you use this pattern. https://codereview.chromium.org/2361283002/diff/220001/ui/views/widget/deskto... File ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc (right): https://codereview.chromium.org/2361283002/diff/220001/ui/views/widget/deskto... ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc:216: TEST_F(DesktopScreenX11Test, GetDisplayById) { Can you clarify why you need the same test for each platform? Aren't you effectively testing the exact same implementation each time? By that I mean Screen:GetDisplayWithDisplayId() is non-virtual, so isn't one test enough?
Patchset #8 (id:240001) has been deleted
riajiang@chromium.org changed reviewers: - dtrainor@chromium.org
-dtrainor@ since no blimp file is modified anymore On 2016/11/19 15:01:40, sky wrote: > https://codereview.chromium.org/2361283002/diff/220001/ui/display/win/screen_... > File ui/display/win/screen_win_unittest.cc (right): > > https://codereview.chromium.org/2361283002/diff/220001/ui/display/win/screen_... > ui/display/win/screen_win_unittest.cc:362: EXPECT_TRUE(has_display); > no need for the temporary. This comment holds for all tests where I think you > use this pattern. > > https://codereview.chromium.org/2361283002/diff/220001/ui/views/widget/deskto... > File ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc (right): > > https://codereview.chromium.org/2361283002/diff/220001/ui/views/widget/deskto... > ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc:216: > TEST_F(DesktopScreenX11Test, GetDisplayById) { > Can you clarify why you need the same test for each platform? Aren't you > effectively testing the exact same implementation each time? By that I mean > Screen:GetDisplayWithDisplayId() is non-virtual, so isn't one test enough? True! These unittests for each platform were from when Screen:GetDisplayWithDisplayId() was still a virtual function. Moved test to screen_unittest.
LGTM
dtrainor@chromium.org changed reviewers: + dtrainor@chromium.org
blimp/ lgtm. Thanks for adding the test!
The CQ bit was checked by riajiang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by riajiang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org, jamescook@chromium.org Link to the patchset: https://codereview.chromium.org/2361283002/#ps260001 (title: "move unittest")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 260001, "attempt_start_ts": 1479758460596240,
"parent_rev": "80a2665628d5d9a7b020b34f9ae0933960d1103e", "commit_rev":
"f6266462809c0c0ea7e37d07f65af015ff05e1f9"}
Message was sent while issue was closed.
Description was changed from
==========
Add GetDisplayWithDisplayId to display::Screen.
BUG=600815
TEST=ash_unittests blimp_unittests app_shell_unittests
display_unittests views_unittests
==========
to
==========
Add GetDisplayWithDisplayId to display::Screen.
BUG=600815
TEST=ash_unittests blimp_unittests app_shell_unittests
display_unittests views_unittests
==========
Message was sent while issue was closed.
Committed patchset #8 (id:260001)
Message was sent while issue was closed.
Description was changed from
==========
Add GetDisplayWithDisplayId to display::Screen.
BUG=600815
TEST=ash_unittests blimp_unittests app_shell_unittests
display_unittests views_unittests
==========
to
==========
Add GetDisplayWithDisplayId to display::Screen.
BUG=600815
TEST=ash_unittests blimp_unittests app_shell_unittests
display_unittests views_unittests
Committed: https://crrev.com/29f6a42748e827c3de474c76d6d2f56a78369d9c
Cr-Commit-Position: refs/heads/master@{#433618}
==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/29f6a42748e827c3de474c76d6d2f56a78369d9c Cr-Commit-Position: refs/heads/master@{#433618} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
