Bug 83851 – [chromium] Move WebVideoFrame into Platform and remove WebCore::VideoFrameChromium wrapper API

• Component: New Bugs
• Status: RESOLVED FIXED

[James Robinson]

[chromium] Move WebVideoFrame into Platform and remove WebCore::VideoFrameChromium wrapper API

[James Robinson]

Created attachment 137024 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adam Barth]

Comment on attachment 137024 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=137024&action=review

> Source/Platform/chromium/src/WebVideoFrame.cpp:43
> +const unsigned WebVideoFrame::maxPlanes = 3;
> +const unsigned WebVideoFrame::numRGBPlanes = 1;
> +const unsigned WebVideoFrame::rgbPlane = 0;
> +const unsigned WebVideoFrame::numYUVPlanes = 3;
> +const unsigned WebVideoFrame::yPlane = 0;
> +const unsigned WebVideoFrame::uPlane = 1;
> +const unsigned WebVideoFrame::vPlane = 2;

We can't put these in the header?

[James Robinson]

They probably could live in the header, but I would have to verify that they don't need storage (or convert them from ints to enums).  Keeping the existing mechanism in place seemed safer since the patch is also doing other things.

[James Robinson]

Committed r114075: <http://trac.webkit.org/changeset/114075>

[James Robinson]

This failed because some .cpp's in webkit_unit_tests #included headers in WebCore that include WebVideoFrame.h as "#include <public/WebVideoFrame.h>", and webkit_unit_tests was being compiled without Source/Platform/chromium/ on the include path.  webkit_unit_tests depend on WebKit.gyp:webkit and nothing else.  Since webkit_unit_tests is an executable, it can't depend directly on webkit_platform since webkit_platform cannot compile as a standalone library - it has to compile as part of webkit.dll since it depends on symbols in WTF that are actually defined in Source/WebKit/chromium/src/...  Since webkit_unit_tests is special in that it can #include WebCore headers while no other users of WebKit.gyp:webkit are allowed to, it seems that the best option is to put the Platform API include path directly on the webkit_unit_test target.

[James Robinson]

Reopening due to revert: http://trac.webkit.org/changeset/114078

[James Robinson]

diff --git a/Source/WebKit/chromium/WebKitUnitTests.gyp b/Source/WebKit/chromium/WebKitUnitTests.gyp
index 4776dc4..6f1ed11 100644
--- a/Source/WebKit/chromium/WebKitUnitTests.gyp
+++ b/Source/WebKit/chromium/WebKitUnitTests.gyp
@@ -72,6 +72,11 @@
             'include_dirs': [
                 'public',
                 'src',
+                # WebKit unit tests are allowed to include WebKit and WebCore header files, which may include headers in the
+                # Platform API as <public/WebFoo.h>. Thus we need to have the Platform API include path, but we can't depend
+                # directly on Platform.gyp:webkit_platform since platform cannot link as a separate library. Instead, we just
+                # add the include path directly.
+                '../../Platform/chromium',
             ],
             'conditions': [
                 ['inside_chromium_build==1 and component=="shared_library"', {

[James Robinson]

Created attachment 137134 [details]
Patch

[James Robinson]

Committed r114164: <http://trac.webkit.org/changeset/114164>