Create gfx::Screen at startup for cast_shell

Create gfx::Screen at startup for cast_shell

Seems like it's incorrect to defer this creation the way we do
currently.

BUG=468362
Committed: https://crrev.com/dc4d8688467495afd4716875e15e3cbee5cd710d
Cr-Commit-Position: refs/heads/master@{#322301}

[halliwell, 2015-03-25 18:57:25]

halliwell@chromium.org changed reviewers:
+ gfhuang@chromium.org, gusfernandez@chromium.org, isherman@chromium.org,
lcwu@chromium.org, oshima@chromium.org

[lcwu1, 2015-03-25 20:32:28]

lgtm % additional comments.

https://codereview.chromium.org/1021483005/diff/1/chromecast/browser/cast_bro...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1021483005/diff/1/chromecast/browser/cast_bro...
chromecast/browser/cast_browser_main_parts.cc:204:
DCHECK(!gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_NATIVE));
Let's add some comments explaining why we need to create a dummy screen here.

[Ilya Sherman, 2015-03-25 20:32:52]

This is far outside my scope of expertise, but the general approach here seems
much better to me.  Thanks!

[halliwell, 2015-03-25 21:56:48]

https://codereview.chromium.org/1021483005/diff/1/chromecast/browser/cast_bro...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1021483005/diff/1/chromecast/browser/cast_bro...
chromecast/browser/cast_browser_main_parts.cc:204:
DCHECK(!gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_NATIVE));
On 2015/03/25 20:32:28, lcwu1 wrote:
> Let's add some comments explaining why we need to create a dummy screen here.

Done.

[oshima, 2015-03-25 22:39:55]

https://codereview.chromium.org/1021483005/diff/20001/chromecast/browser/cast...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1021483005/diff/20001/chromecast/browser/cast...
chromecast/browser/cast_browser_main_parts.cc:210:
aura::TestScreen::Create(gfx::Size(0, 0)));
Hmm, production code should not depend on test code, but looks like this is
already used in cast. lgtm for now, but could you please create cast's screen
and remove the dependency to test?

[halliwell, 2015-03-26 00:28:24]

https://codereview.chromium.org/1021483005/diff/20001/chromecast/browser/cast...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1021483005/diff/20001/chromecast/browser/cast...
chromecast/browser/cast_browser_main_parts.cc:210:
aura::TestScreen::Create(gfx::Size(0, 0)));
On 2015/03/25 22:39:55, oshima wrote:
> Hmm, production code should not depend on test code, but looks like this is
> already used in cast. lgtm for now, but could you please create cast's screen
> and remove the dependency to test?

Yes, sure.  I filed https://code.google.com/p/chromium/issues/detail?id=470735
to follow up on this.

And to clarify - should we just create our own Screen implementation?  We have a
TODO in CastContentWindow::CreateWindowTree that suggests "use ozone's screen
implementation when it is ready" ... does that still apply?

[oshima, 2015-03-26 00:46:48]

On 2015/03/26 00:28:24, halliwell wrote:
>
https://codereview.chromium.org/1021483005/diff/20001/chromecast/browser/cast...
> File chromecast/browser/cast_browser_main_parts.cc (right):
> 
>
https://codereview.chromium.org/1021483005/diff/20001/chromecast/browser/cast...
> chromecast/browser/cast_browser_main_parts.cc:210:
> aura::TestScreen::Create(gfx::Size(0, 0)));
> On 2015/03/25 22:39:55, oshima wrote:
> > Hmm, production code should not depend on test code, but looks like this is
> > already used in cast. lgtm for now, but could you please create cast's
screen
> > and remove the dependency to test?
> 
> Yes, sure.  I filed https://code.google.com/p/chromium/issues/detail?id=470735
> to follow up on this.
> 
> And to clarify - should we just create our own Screen implementation?  We have
a
> TODO in CastContentWindow::CreateWindowTree that suggests "use ozone's screen
> implementation when it is ready" ... does that still apply?

ozone is lower level than gfx::Screen. It's possible for ozone to provide
minimum implementation 
(I don't know if they will), so if they do (and it satisfies you needs), then
sure, you can do that.

[halliwell, 2015-03-26 01:10:15]

The CQ bit was checked by halliwell@chromium.org

[halliwell, 2015-03-26 01:10:15]

The patchset sent to the CQ was uploaded after l-g-t-m from lcwu@chromium.org
Link to the patchset: https://codereview.chromium.org/1021483005/#ps20001
(title: "Add explanatory comment")

[halliwell, 2015-03-26 01:11:14]

On 2015/03/26 00:46:48, oshima wrote:
> On 2015/03/26 00:28:24, halliwell wrote:
> >
>
https://codereview.chromium.org/1021483005/diff/20001/chromecast/browser/cast...
> > File chromecast/browser/cast_browser_main_parts.cc (right):
> > 
> >
>
https://codereview.chromium.org/1021483005/diff/20001/chromecast/browser/cast...
> > chromecast/browser/cast_browser_main_parts.cc:210:
> > aura::TestScreen::Create(gfx::Size(0, 0)));
> > On 2015/03/25 22:39:55, oshima wrote:
> > > Hmm, production code should not depend on test code, but looks like this
is
> > > already used in cast. lgtm for now, but could you please create cast's
> screen
> > > and remove the dependency to test?
> > 
> > Yes, sure.  I filed
https://code.google.com/p/chromium/issues/detail?id=470735
> > to follow up on this.
> > 
> > And to clarify - should we just create our own Screen implementation?  We
have
> a
> > TODO in CastContentWindow::CreateWindowTree that suggests "use ozone's
screen
> > implementation when it is ready" ... does that still apply?
> 
> ozone is lower level than gfx::Screen. It's possible for ozone to provide
> minimum implementation 
> (I don't know if they will), so if they do (and it satisfies you needs), then
> sure, you can do that.

Gotcha, thanks :)

[commit-bot: I haz the power, 2015-03-26 01:11:18]

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

[commit-bot: I haz the power, 2015-03-26 02:21:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-26 02:22:35]

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