Relocate last remnants of webkit/renderer/media code.

Relocate last remnants of webkit/renderer/media code.

- Moves webkit/renderer/media/crypto/ppapi/ to media/cdm/ppapi/
- Moves webkit/.../ppapi/cdm/ to media/cdm/ppapi/api/
- Moves everything in webkit_media to media namespace
- Fixes broken builds with use_fake_video_decoder=1 or use_libvpx=1
- Deletes webkit/renderer/media folder (\o/)

BUG=251306
TEST=Chrome builds with mentioned flags, plus branding=Chrome.
TBR=rsleevi

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

[DaleCurtis, 2013-08-09 00:40:58]

ddorwin: everything
jamesr: webkit/ changes
jam: content_tests.gypi change.

[ddorwin, 2013-08-09 01:10:38]

Thanks! LGTM with comments.

I suggest changing the description to "webkit/renderer/media/" instead of
"webkit_media" since the former is the important part and what we've been using
for other CLs.

https://codereview.chromium.org/22362007/diff/1/media/cdm/ppapi/DEPS
File media/cdm/ppapi/DEPS (right):

https://codereview.chromium.org/22362007/diff/1/media/cdm/ppapi/DEPS#newcode1
media/cdm/ppapi/DEPS:1: include_rules = [
We might want to note that this is not a dependency or used by media/ so we can
take these extra dependencies.

https://codereview.chromium.org/22362007/diff/1/media/cdm/ppapi/libvpx_cdm_vi...
File media/cdm/ppapi/libvpx_cdm_video_decoder.cc (right):

https://codereview.chromium.org/22362007/diff/1/media/cdm/ppapi/libvpx_cdm_vi...
media/cdm/ppapi/libvpx_cdm_video_decoder.cc:17: // Note: vpx_decoder.h must be
first or compile will fail.
Should we file a bug against libvpx for IWYU?

[jamesr, 2013-08-09 01:15:47]

Can you remove the +crypto line from src/webkit/DEPS and from
webkit/support/webkit_support.gypi, or is something else using those?

[jam, 2013-08-09 15:51:38]

lgtm

[DaleCurtis, 2013-08-09 18:29:38]

PTAL:
- Updated title and description per ddorwin@
- Removed crypto from webkit DEPS and gypi per jamesr@

https://codereview.chromium.org/22362007/diff/1/media/cdm/ppapi/DEPS
File media/cdm/ppapi/DEPS (right):

https://codereview.chromium.org/22362007/diff/1/media/cdm/ppapi/DEPS#newcode1
media/cdm/ppapi/DEPS:1: include_rules = [
On 2013/08/09 01:10:38, ddorwin wrote:
> We might want to note that this is not a dependency or used by media/ so we
can
> take these extra dependencies.

Done.

https://codereview.chromium.org/22362007/diff/1/media/cdm/ppapi/libvpx_cdm_vi...
File media/cdm/ppapi/libvpx_cdm_video_decoder.cc (right):

https://codereview.chromium.org/22362007/diff/1/media/cdm/ppapi/libvpx_cdm_vi...
media/cdm/ppapi/libvpx_cdm_video_decoder.cc:17: // Note: vpx_decoder.h must be
first or compile will fail.
On 2013/08/09 01:10:38, ddorwin wrote:
> Should we file a bug against libvpx for IWYU?

Filed https://code.google.com/p/webm/issues/detail?id=598

[jamesr, 2013-08-09 18:31:02]

Cool, webkit/ lgtm

[commit-bot: I haz the power, 2013-08-09 19:18:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/22362007/8001

[commit-bot: I haz the power, 2013-08-09 19:19:12]

Failed to apply patch for webkit/support/webkit_support.gyp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file webkit/support/webkit_support.gyp
  Hunk #1 FAILED at 12.
  1 out of 1 hunk FAILED -- saving rejects to file
webkit/support/webkit_support.gyp.rej

Patch:       webkit/support/webkit_support.gyp
Index: webkit/support/webkit_support.gyp
diff --git a/webkit/support/webkit_support.gyp
b/webkit/support/webkit_support.gyp
index
7e3974775ef39c982cba66446c5cbc097dd920f1..e8895d477be12e5e7078b39ebb39741a0023bbdd
100644
--- a/webkit/support/webkit_support.gyp
+++ b/webkit/support/webkit_support.gyp
@@ -12,7 +12,6 @@
         '../../build/win_precompile.gypi',
         '../glue/webkit_glue.gypi',
         '../plugins/webkit_plugins.gypi',
-        '../renderer/media/webkit_media.gypi',
         'webkit_support.gypi',
       ],
     }],

[commit-bot: I haz the power, 2013-08-09 19:38:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/22362007/15001

[DaleCurtis, 2013-08-09 20:21:41]

+dmichael for ppapi DEPS owners approval for movement from
webkit/renderer/media/crypto/ppapi to media/cdm/ppapi/

TBR=rsleevi for directory rename which presubmit wants a DEPS owners approval
for.

[dmichael (off chromium), 2013-08-09 20:25:55]

https://codereview.chromium.org/22362007/diff/21001/media/cdm/ppapi/DEPS
File media/cdm/ppapi/DEPS (right):

https://codereview.chromium.org/22362007/diff/21001/media/cdm/ppapi/DEPS#newc...
media/cdm/ppapi/DEPS:4: "+ppapi",
there's some stuff in ppapi that media/cdm/ppapi probably should not depend on
(e.g., proxy, shared_impl).

It may be better to include only the few things that you need, like:
ppapi/c
ppapi/cpp
ppapi/utility

(I think that would cover it)

[jamesr, 2013-08-09 20:29:59]

On 2013/08/09 20:21:41, DaleCurtis wrote:
> +dmichael for ppapi DEPS owners approval for movement from
> webkit/renderer/media/crypto/ppapi to media/cdm/ppapi/
> 
> TBR=rsleevi for directory rename which presubmit wants a DEPS owners approval
> for.

Don't TBR DEPS changes, they're important to review carefully.

I don't think adding a "+ppapi" under media/ makes sense since ppapi/DEPS has
"+media".  You need to make these more specific to break the cycle.

[dmichael (off chromium), 2013-08-09 20:38:43]

On 2013/08/09 20:29:59, jamesr wrote:
> On 2013/08/09 20:21:41, DaleCurtis wrote:
> > +dmichael for ppapi DEPS owners approval for movement from
> > webkit/renderer/media/crypto/ppapi to media/cdm/ppapi/
> > 
> > TBR=rsleevi for directory rename which presubmit wants a DEPS owners
approval
> > for.
> 
> Don't TBR DEPS changes, they're important to review carefully.
> 
> I don't think adding a "+ppapi" under media/ makes sense since ppapi/DEPS has
> "+media".  You need to make these more specific to break the cycle.

Good point. Probabably, that should be removed. We can add it to
ppapi/proxy/DEPS and ppapi/shared_impl/DEPS.

[DaleCurtis, 2013-08-09 20:41:39]

TBR'd since the crypto/ DEPS was already in media/crypto which was renamed to
media/cdm.  Vice versa, didn't TBR dmichael for ppapi since its an actual top
level move.

The media DEP in ppapi is for some select bits of media functionality which are
in media/audio and media/base.  We can restrict the set to those DEPS if
dmichael prefers.

https://codereview.chromium.org/22362007/diff/21001/media/cdm/ppapi/DEPS
File media/cdm/ppapi/DEPS (right):

https://codereview.chromium.org/22362007/diff/21001/media/cdm/ppapi/DEPS#newc...
media/cdm/ppapi/DEPS:4: "+ppapi",
On 2013/08/09 20:25:56, dmichael wrote:
> there's some stuff in ppapi that media/cdm/ppapi probably should not depend on
> (e.g., proxy, shared_impl).
> 
> It may be better to include only the few things that you need, like:
> ppapi/c
> ppapi/cpp
> ppapi/utility
> 
> (I think that would cover it)

Done.

[Ryan Sleevi, 2013-08-09 20:43:24]

How'd I get roped in? DEPS is *, and I'm not an OWNER of the other directories
AFAIK

[DaleCurtis, 2013-08-09 20:45:38]

rsleevi: media/crypto/DEPS had a +crypto DEP and I moved it to media/cdm, as it
seems like a new DEPS, presubmit wants a crypto owner to approve.

[DaleCurtis, 2013-08-09 20:46:36]

dmichael: Latest patch set restricts both media and ppapi deps.

[Ryan Sleevi, 2013-08-09 20:47:59]

Ah. In that case, LGTM.

[dmichael (off chromium), 2013-08-09 20:55:39]

lgtm

[commit-bot: I haz the power, 2013-08-09 21:03:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/22362007/35001

[commit-bot: I haz the power, 2013-08-09 21:57:36]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[DaleCurtis, 2013-08-09 22:38:49]

Looks like chromeos/ had an implicit dependency on crypto/ +stevenjb to review
chromeos.gyp change making this explicit.

[stevenjb, 2013-08-09 23:52:24]

chromeos/chromeos.gyp change LGTM

[commit-bot: I haz the power, 2013-08-09 23:58:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/22362007/31002

[commit-bot: I haz the power, 2013-08-10 08:21:32]

Change committed as 216826