Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
TBR=watk@chromium.org, mkwst@chromium.org, sky@chromium.org, stevenjb@chromium.org

Committed: https://crrev.com/a15deb00fe322494ce3830143a5f3e705ecdc577
Cr-Commit-Position: refs/heads/master@{#392783}

[oshima, 2016-05-10 20:55:28]

Description was changed from

==========
Move gfx::Screen and its impl to ui/display

BUG=597761
==========

to

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
==========

[oshima, 2016-05-10 20:55:48]

The CQ bit was checked by oshima@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-10 20:56:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1964153002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1964153002/1

[oshima, 2016-05-10 21:09:26]

oshima@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2016-05-10 21:34:37]

lgtm

https://codereview.chromium.org/1964153002/diff/1/ui/display/BUILD.gn
File ui/display/BUILD.gn (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/BUILD.gn#newcode199
ui/display/BUILD.gn:199: sources -= [ "screen_unittest.cc" ]
Why is this excluded here?

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp
File ui/display/display.gyp (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp#newc...
ui/display/display.gyp:49: 'android/screen_android.cc',
indent

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp#newc...
ui/display/display.gyp:258: 'screen_unittest.cc',
same question

https://codereview.chromium.org/1964153002/diff/1/ui/display/display_observer.h
File ui/display/display_observer.h (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/display_observer...
ui/display/display_observer.h:46: };
private: DISALLOW_...

[oshima, 2016-05-10 22:00:51]

https://codereview.chromium.org/1964153002/diff/1/ui/display/BUILD.gn
File ui/display/BUILD.gn (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/BUILD.gn#newcode199
ui/display/BUILD.gn:199: sources -= [ "screen_unittest.cc" ]
On 2016/05/10 21:34:37, scottmg wrote:
> Why is this excluded here?

This is minimum tests for non aura platforms. Here is the content:

https://code.google.com/p/chromium/codesearch#chromium/src/ui/gfx/screen_unit...

https://codereview.chromium.org/1964153002/diff/1/ui/display/display_observer.h
File ui/display/display_observer.h (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/display_observer...
ui/display/display_observer.h:46: };
On 2016/05/10 21:34:37, scottmg wrote:
> private: DISALLOW_...

It is not required for pure virual interface.

https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/js024SU2lTs

I can add if you want though. Please let me know.

[scottmg, 2016-05-10 22:06:43]

https://codereview.chromium.org/1964153002/diff/1/ui/display/display_observer.h
File ui/display/display_observer.h (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/display_observer...
ui/display/display_observer.h:46: };
On 2016/05/10 22:00:51, oshima wrote:
> On 2016/05/10 21:34:37, scottmg wrote:
> > private: DISALLOW_...
> 
> It is not required for pure virual interface.
> 
>
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/js024SU2lTs
> 
> I can add if you want though. Please let me know.

Shrug, doesn't matter.

[oshima, 2016-05-10 22:15:28]

Description was changed from

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
==========

to

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
TBR=watk@chromium.org, mkwst@chromium.org, sky@chromium.org
==========

[oshima, 2016-05-10 22:15:29]

oshima@chromium.org changed reviewers:
+ mkwst@chromium.org, sky@chromium.org, watk@chromium.org

[oshima, 2016-05-10 22:16:41]

Description was changed from

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
TBR=watk@chromium.org, mkwst@chromium.org, sky@chromium.org
==========

to

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
TBR=watk@chromium.org, mkwst@chromium.org, sky@chromium.org,
stevenjb@chromium.org
==========

[oshima, 2016-05-10 22:16:42]

oshima@chromium.org changed reviewers:
+ stevenjb@chromium.org

[oshima, 2016-05-10 22:17:49]

tbr'ing trivial changes in the following directories

mkwst@ -> content/shell
watk@ -> media/
sky@ -> ui/views
stevenjb@ -> ui/message_center

[stevenjb, 2016-05-10 22:26:39]

lgtm

[oshima, 2016-05-10 22:32:22]

The CQ bit was unchecked by oshima@chromium.org

[oshima, 2016-05-10 22:32:30]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2016-05-10 22:33:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1964153002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1964153002/1

[oshima, 2016-05-10 22:36:11]

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp
File ui/display/display.gyp (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp#newc...
ui/display/display.gyp:49: 'android/screen_android.cc',
On 2016/05/10 21:34:37, scottmg wrote:
> indent

forgot to respond. This looks fine. Am I missing something?

[scottmg, 2016-05-10 22:42:19]

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp
File ui/display/display.gyp (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp#newc...
ui/display/display.gyp:49: 'android/screen_android.cc',
On 2016/05/10 22:36:11, oshima wrote:
> On 2016/05/10 21:34:37, scottmg wrote:
> > indent
> 
> forgot to respond. This looks fine. Am I missing something?

It looks like it's two spaces too far to the right to me. Same with
ios/screen_ios.mm, and mac/screen_mac.mm below.

[scottmg, 2016-05-10 22:43:19]

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp
File ui/display/display.gyp (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp#newc...
ui/display/display.gyp:49: 'android/screen_android.cc',
On 2016/05/10 22:42:19, scottmg wrote:
> On 2016/05/10 22:36:11, oshima wrote:
> > On 2016/05/10 21:34:37, scottmg wrote:
> > > indent
> > 
> > forgot to respond. This looks fine. Am I missing something?
> 
> It looks like it's two spaces too far to the right to me. Same with
> ios/screen_ios.mm, and mac/screen_mac.mm below.

Oh, it's actually that they're tabs. I don't know what happened to the red > we
used to have. :(

[oshima, 2016-05-10 22:45:48]

The CQ bit was unchecked by oshima@chromium.org

[oshima, 2016-05-10 22:51:54]

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp
File ui/display/display.gyp (right):

https://codereview.chromium.org/1964153002/diff/1/ui/display/display.gyp#newc...
ui/display/display.gyp:49: 'android/screen_android.cc',
On 2016/05/10 22:43:19, scottmg wrote:
> On 2016/05/10 22:42:19, scottmg wrote:
> > On 2016/05/10 22:36:11, oshima wrote:
> > > On 2016/05/10 21:34:37, scottmg wrote:
> > > > indent
> > > 
> > > forgot to respond. This looks fine. Am I missing something?
> > 
> > It looks like it's two spaces too far to the right to me. Same with
> > ios/screen_ios.mm, and mac/screen_mac.mm below.
> 
> Oh, it's actually that they're tabs. I don't know what happened to the red >
we
> used to have. :(

Oh, they were indeed tabs. Thank you for catching them.

[oshima, 2016-05-10 22:52:21]

The CQ bit was checked by oshima@chromium.org

[oshima, 2016-05-10 22:52:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/1964153002/#ps20001
(title: "fix tabs")

[commit-bot: I haz the power, 2016-05-10 22:52:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1964153002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1964153002/20001

[commit-bot: I haz the power, 2016-05-11 00:19:43]

Description was changed from

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
TBR=watk@chromium.org, mkwst@chromium.org, sky@chromium.org,
stevenjb@chromium.org
==========

to

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
TBR=watk@chromium.org, mkwst@chromium.org, sky@chromium.org,
stevenjb@chromium.org
==========

[commit-bot: I haz the power, 2016-05-11 00:19:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-11 00:22:10]

Description was changed from

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
TBR=watk@chromium.org, mkwst@chromium.org, sky@chromium.org,
stevenjb@chromium.org
==========

to

==========
Move Screen, its impls, DisplayObserver and DisplayChangeNotifier to ui/display

BUG=597761
TBR=watk@chromium.org, mkwst@chromium.org, sky@chromium.org,
stevenjb@chromium.org

Committed: https://crrev.com/a15deb00fe322494ce3830143a5f3e705ecdc577
Cr-Commit-Position: refs/heads/master@{#392783}
==========

[commit-bot: I haz the power, 2016-05-11 00:22:12]

Patchset 2 (id:??) landed as
https://crrev.com/a15deb00fe322494ce3830143a5f3e705ecdc577
Cr-Commit-Position: refs/heads/master@{#392783}

[chcunningham, 2016-05-18 17:44:47]

On 2016/05/11 00:22:12, commit-bot: I haz the power wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/a15deb00fe322494ce3830143a5f3e705ecdc577
> Cr-Commit-Position: refs/heads/master@{#392783}

Did this break gyp? I'm getting this error now:

ninja: error: '../../ui/gfx/android/screen_android.cc', needed by
'obj/ui/gfx/android/gfx.screen_android.o', missing and no known rule to make it

I know I know... gyp is deprecated. I'm building blink_tests for android (gn
support is not quite there for this target yet).

[watk, 2016-05-18 18:15:16]

On 2016/05/18 17:44:47, chcunningham wrote:
> On 2016/05/11 00:22:12, commit-bot: I haz the power wrote:
> > Patchset 2 (id:??) landed as
> > https://crrev.com/a15deb00fe322494ce3830143a5f3e705ecdc577
> > Cr-Commit-Position: refs/heads/master@{#392783}
> 
> Did this break gyp? I'm getting this error now:
> 
> ninja: error: '../../ui/gfx/android/screen_android.cc', needed by
> 'obj/ui/gfx/android/gfx.screen_android.o', missing and no known rule to make
it
> 
> I know I know... gyp is deprecated. I'm building blink_tests for android (gn
> support is not quite there for this target yet).

Looks like android/screen_android.cc needs to be deleted from ui/gfx/gfx.gyp?

[oshima, 2016-05-19 18:22:48]

On 2016/05/18 18:15:16, watk wrote:
> On 2016/05/18 17:44:47, chcunningham wrote:
> > On 2016/05/11 00:22:12, commit-bot: I haz the power wrote:
> > > Patchset 2 (id:??) landed as
> > > https://crrev.com/a15deb00fe322494ce3830143a5f3e705ecdc577
> > > Cr-Commit-Position: refs/heads/master@{#392783}
> > 
> > Did this break gyp? I'm getting this error now:
> > 
> > ninja: error: '../../ui/gfx/android/screen_android.cc', needed by
> > 'obj/ui/gfx/android/gfx.screen_android.o', missing and no known rule to make
> it
> > 
> > I know I know... gyp is deprecated. I'm building blink_tests for android (gn
> > support is not quite there for this target yet).
> 
> Looks like android/screen_android.cc needs to be deleted from ui/gfx/gfx.gyp?

https://codereview.chromium.org/1994253002/ sent. Sorry for trouble.