Fixit: Move tab and desktop capture code to new location.

Fixit: Move tab and desktop capture code to new location.

The purpose of this change is to remove a layering violation: Code in content/browser/renderer_host should not depend on content/browser/web_contents.  As a secondary benefit, the code modules have been consolidated into a more-appropriate location, with cleaner DEPS/OWNERS files.

There are NO CODE CHANGES being made here, other than include's and ifdef guards in .h files.

BUG=178495
R=fbarchard@chromium.org, jam@chromium.org, scherkus@chromium.org

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

[miu, 2014-02-20 03:47:30]

Clean-up time!  :)

fbarchard: approval for adding libyuv to content/browser/media/capture/DEPS

jam: OWNERS approval for content/browser/browser_main_loop.cc

scherkus: approval of the rest (what I moved, and where I moved it to).  Please
take a look at the OWNERS/DEPS files I've added and changed.

[scherkus (not reviewing), 2014-02-20 18:46:55]

OOC can we remove this exclusion jam@ added with this CL?
https://codereview.chromium.org/12319119/diff/1/content/browser/renderer_host...

https://codereview.chromium.org/172003004/diff/20001/content/browser/media/OW...
File content/browser/media/OWNERS (right):

https://codereview.chromium.org/172003004/diff/20001/content/browser/media/OW...
content/browser/media/OWNERS:14: # capture/OWNERS for code reviews.
is this needed? I'd imagine folks touching capture/ code would look at
capture/OWNERS

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

https://codereview.chromium.org/172003004/diff/20001/content/browser/renderer...
content/browser/renderer_host/media/audio_input_renderer_host.cc:281:
WebContentsAudioInputStream::Create(
I forget the details of the renderer_host-can't-depend-on-web_contents
discussion ... but is it still considered a layering violaton if
renderer_host/media/ code is able to use web_contents code via these classes?

[miu, 2014-02-20 21:29:02]

jam: Can you comment on scherkus' concerns w.r.t.
content/browser/renderer_host/DEPS and why this code shouldn't depend directly
on web_contents?

On 2014/02/20 18:46:55, scherkus wrote:
> OOC can we remove this exclusion jam@ added with this CL?
>
https://codereview.chromium.org/12319119/diff/1/content/browser/renderer_host...

jam@ would be better to answer this question.  My understanding of this is
stated in response to one of your comments below:

https://codereview.chromium.org/172003004/diff/20001/content/browser/media/OW...
File content/browser/media/OWNERS (right):

https://codereview.chromium.org/172003004/diff/20001/content/browser/media/OW...
content/browser/media/OWNERS:14: # capture/OWNERS for code reviews.
On 2014/02/20 18:46:56, scherkus wrote:
> is this needed? I'd imagine folks touching capture/ code would look at
> capture/OWNERS

Done.

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

https://codereview.chromium.org/172003004/diff/20001/content/browser/renderer...
content/browser/renderer_host/media/audio_input_renderer_host.cc:281:
WebContentsAudioInputStream::Create(
On 2014/02/20 18:46:56, scherkus wrote:
> I forget the details of the renderer_host-can't-depend-on-web_contents
> discussion ... but is it still considered a layering violaton if
> renderer_host/media/ code is able to use web_contents code via these classes?

I forget the details too.  ;-)  My understanding, based on looking at the bug
and code history, is that the restriction was put in-place to drive clean code
organization rather than force the object dependency graph.

In fact, we can never force such an object dependency graph without adding
[useless] layers of indirection, since tab capture is literally the recording of
audio and video from a "WebContents."

[fbarchard, 2014-02-22 01:50:02]

lgtm

[fbarchard, 2014-02-22 01:50:04]

lgtm

[jabdelmalek, 2014-02-24 18:21:00]

On 2014/02/20 21:29:02, miu wrote:
> jam: Can you comment on scherkus' concerns w.r.t.
> content/browser/renderer_host/DEPS and why this code shouldn't depend directly
> on web_contents?
> 
> On 2014/02/20 18:46:55, scherkus wrote:
> > OOC can we remove this exclusion jam@ added with this CL?
> >
>
https://codereview.chromium.org/12319119/diff/1/content/browser/renderer_host...
> 

the exclusion was for tests, which still need it, and that's ok.

so it wouldn't be possible to remove that.

[jabdelmalek, 2014-02-24 19:19:12]

lgtm for files not in media directories

for the DEPS rule, the reason is that RVH sits below WebContents. So it
shouldn't know anything about it. Instead it talks to it through
RenderViewHostDelegate.

[scherkus (not reviewing), 2014-02-24 22:26:10]

lgtm

[miu, 2014-02-24 23:00:36]

The CQ bit was checked by miu@chromium.org

[commit-bot: I haz the power, 2014-02-24 23:05:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miu@chromium.org/172003004/600001

[commit-bot: I haz the power, 2014-02-24 23:39:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-24 23:39:07]

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

[miu, 2014-02-25 00:05:05]

The CQ bit was checked by miu@chromium.org

[miu, 2014-02-25 00:06:02]

The CQ bit was unchecked by miu@chromium.org

[miu, 2014-02-25 00:07:47]

The CQ bit was checked by miu@chromium.org

[jam, 2014-02-25 00:09:54]

lgtm

[commit-bot: I haz the power, 2014-02-25 00:11:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miu@chromium.org/172003004/600001

[commit-bot: I haz the power, 2014-02-25 01:45:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miu@chromium.org/172003004/600001

[miu, 2014-02-25 02:07:04]

The CQ is acting really wonky, this has passed all trybots (in PS1), and this is
now blocking a teammate.  So, I'm going to dcommit.

[miu, 2014-02-25 02:08:59]

Committed patchset #3 manually as r253058 (presubmit successful).