Add AshDesktopMediaList and enable Desktop Capture API on Chrome OS.

Add AshDesktopMediaList and enable Desktop Capture API on Chrome OS.

The new AshDesktopMediaList gets list of windows/screens for the window
picker dialog which allows to enable Desktop Capture API on Chrome OS.

BUG=295102
R=ben@chromium.org, hshi@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242361

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242578

[jam, 2013-12-05 20:49:46]

Which parts do you want me to look at? the gypi files have all devs as owners,
the other files are in media or extensions directories and you should get owners
from there to take a look as they'd be more familiar than me.

For the DEPS, adding jamesr: I'm not sure if they want chrome to include cc or
not.

[Sergey Ulanov, 2013-12-05 20:53:41]

On 2013/12/05 20:49:46, jam wrote:
> Which parts do you want me to look at? the gypi files have all devs as owners,
> the other files are in media or extensions directories and you should get
owners
> from there to take a look as they'd be more familiar than me.
> 
> For the DEPS, adding jamesr: I'm not sure if they want chrome to include cc or
> not.

Sorry for not being clear about it. I need your approval for the DEPS change and
also I'm not an owner of chrome/browser/ui/views/desktop_media_picker_views.cc

jamesr: can you please approve the new dependency on src/cc

[jamesr, 2013-12-05 21:38:08]

I'm not sure exactly where this code should be but its current location feels
misplaced. It's poking at the implementation of the aura::Window to read pixels
out, but it can't use the normal mechanisms we use for this that can do things
like scale on the gpu.

Why is this code living in chrome/ at all?  Thumbnailing an aura window seems
like something that perhaps aura should do.

[jamesr, 2013-12-05 21:38:08]

I'm not sure exactly where this code should be but its current location feels
misplaced. It's poking at the implementation of the aura::Window to read pixels
out, but it can't use the normal mechanisms we use for this that can do things
like scale on the gpu.

Why is this code living in chrome/ at all?  Thumbnailing an aura window seems
like something that perhaps aura should do.

[Sergey Ulanov, 2013-12-05 22:33:08]

On 2013/12/05 21:38:08, jamesr wrote:
> I'm not sure exactly where this code should be but its current location feels
> misplaced. It's poking at the implementation of the aura::Window to read
pixels
> out, but it can't use the normal mechanisms we use for this that can do things
> like scale on the gpu.
> 
> Why is this code living in chrome/ at all?  Thumbnailing an aura window seems
> like something that perhaps aura should do.


DesktopMediaList interface is there just for the window picker UI, it wouldn't
make sense in aura layer. Potentially the thumbnail code can be moved to Aura.

+sky, ben.
Scott, Ben, do you think thumbnailing logic would fit aura layer?

[Ben Goodger (Google), 2013-12-06 18:27:05]

On 2013/12/05 22:33:08, Sergey Ulanov wrote:
> On 2013/12/05 21:38:08, jamesr wrote:
> > I'm not sure exactly where this code should be but its current location
feels
> > misplaced. It's poking at the implementation of the aura::Window to read
> pixels
> > out, but it can't use the normal mechanisms we use for this that can do
things
> > like scale on the gpu.
> > 
> > Why is this code living in chrome/ at all?  Thumbnailing an aura window
seems
> > like something that perhaps aura should do.
> 
> 
> DesktopMediaList interface is there just for the window picker UI, it wouldn't
> make sense in aura layer. Potentially the thumbnail code can be moved to Aura.
> 
> +sky, ben.
> Scott, Ben, do you think thumbnailing logic would fit aura layer?

Aura is *just* a window type (and associated paint scheduling/event
dispatching). Pretty much everything else should be built on top of it.

[hshi1, 2013-12-13 01:28:50]

https://codereview.chromium.org/100833002/diff/20001/chrome/browser/media/ash...
File chrome/browser/media/ash_desktop_media_list.cc (right):

https://codereview.chromium.org/100833002/diff/20001/chrome/browser/media/ash...
chrome/browser/media/ash_desktop_media_list.cc:42: libyuv::kFilterBilinear);
I am not familiar enough with libyuv and skia but it seems strange to use libyuv
to scale SkBitmaps. Shouldn't there be a skia routine to scale SkBitmaps?

https://codereview.chromium.org/100833002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/media/desktop_capture_device_ash.cc (right):

https://codereview.chromium.org/100833002/diff/20001/content/browser/renderer...
content/browser/renderer_host/media/desktop_capture_device_ash.cc:1: //
Copyright 2013 The Chromium Authors. All rights reserved.
This file has already been renamed to desktop_capture_device_aura.cc -- it would
be great if you rebase and resolve to ToT.

[Sergey Ulanov, 2013-12-13 02:54:29]

https://codereview.chromium.org/100833002/diff/20001/chrome/browser/media/ash...
File chrome/browser/media/ash_desktop_media_list.cc (right):

https://codereview.chromium.org/100833002/diff/20001/chrome/browser/media/ash...
chrome/browser/media/ash_desktop_media_list.cc:42: libyuv::kFilterBilinear);
On 2013/12/13 01:28:51, hshi1 wrote:
> I am not familiar enough with libyuv and skia but it seems strange to use
libyuv
> to scale SkBitmaps. Shouldn't there be a skia routine to scale SkBitmaps? 

Done. I used libyuv for scaling here because that's what we use in
content::DesktopCaptureDevice (if I remember correctly we use it there because
it's provides better quality/performance tradeoff, also DesktopCaptureDevice
doesn't use skia). I agree that skia is more appropriate here. It also makes
code cleaner.

https://codereview.chromium.org/100833002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/media/desktop_capture_device_ash.cc (right):

https://codereview.chromium.org/100833002/diff/20001/content/browser/renderer...
content/browser/renderer_host/media/desktop_capture_device_ash.cc:1: //
Copyright 2013 The Chromium Authors. All rights reserved.
On 2013/12/13 01:28:51, hshi1 wrote:
> This file has already been renamed to desktop_capture_device_aura.cc -- it
would
> be great if you rebase and resolve to ToT.

Done.

[hshi1, 2013-12-13 17:56:16]

I'm not on OWNERS for any of these files, but LGTM for browser/media and
content/browser/renderer_host/media

[Sergey Ulanov, 2013-12-19 03:27:19]

PTAL - moved code that creates window thumbnails to ui/snapshot

jam@: change in content/public
jamesr@: new dependency on cc in ui/snapshot
sky@, or ben@: changes in ui/snapshot and chrome/browser/ui

[Ben Goodger (Google), 2013-12-19 04:01:06]

https://codereview.chromium.org/100833002/diff/80001/chrome/browser/ui/views/...
File chrome/browser/ui/views/desktop_media_picker_views.cc (right):

https://codereview.chromium.org/100833002/diff/80001/chrome/browser/ui/views/...
chrome/browser/ui/views/desktop_media_picker_views.cc:506: if
(ash::Shell::HasInstance()) {
instead of doing this, you probably should instead check that this widget's
native window exists within the ash environment

[Sergey Ulanov, 2013-12-20 01:43:07]

https://codereview.chromium.org/100833002/diff/80001/chrome/browser/ui/views/...
File chrome/browser/ui/views/desktop_media_picker_views.cc (right):

https://codereview.chromium.org/100833002/diff/80001/chrome/browser/ui/views/...
chrome/browser/ui/views/desktop_media_picker_views.cc:506: if
(ash::Shell::HasInstance()) {
On 2013/12/19 04:01:07, Ben Goodger (Google) wrote:
> instead of doing this, you probably should instead check that this widget's
> native window exists within the ash environment

Done.

[Ben Goodger (Google), 2013-12-20 21:29:36]

lgtm

[commit-bot: I haz the power, 2013-12-20 22:07:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/100833002/140001

[commit-bot: I haz the power, 2013-12-21 00:46:20]

Retried try job too often on win_x64_rel for step(s) base_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[commit-bot: I haz the power, 2013-12-21 01:45:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/100833002/580001

[commit-bot: I haz the power, 2013-12-21 02:03:45]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-12-21 02:30:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/100833002/790001

[commit-bot: I haz the power, 2013-12-21 03:04:17]

Retried try job too often on ios_dbg_simulator for step(s) base_unittests,
compile, content_unittests, crypto_unittests, net_unittests, sql_unittests,
ui_unittests, url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-12-21 20:05:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/100833002/980001

[commit-bot: I haz the power, 2013-12-21 20:36:59]

Retried try job too often on ios_dbg_simulator for step(s) base_unittests,
compile, content_unittests, crypto_unittests, net_unittests, sql_unittests,
ui_unittests, url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[commit-bot: I haz the power, 2013-12-22 01:09:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/100833002/980001

[commit-bot: I haz the power, 2013-12-22 01:34:52]

Retried try job too often on ios_dbg_simulator for step(s) base_unittests,
compile, content_unittests, crypto_unittests, net_unittests, sql_unittests,
ui_unittests, url_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[Sergey Ulanov, 2013-12-23 06:16:16]

Committed patchset #12 manually as r242361 (presubmit successful).

[commit-bot: I haz the power, 2013-12-26 21:56:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/100833002/1820001

[Sergey Ulanov, 2013-12-27 00:13:52]

Committed patchset #13 manually as r242578 (presubmit successful).