Bug 206043 - [Media in GPU process] Implement the remote video layer support
Summary: [Media in GPU process] Implement the remote video layer support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Peng Liu
URL:
Keywords: InRadar
Depends on:
Blocks: 205123 206132
  Show dependency treegraph
 
Reported: 2020-01-09 17:26 PST by Peng Liu
Modified: 2020-01-14 19:15 PST (History)
16 users (show)

See Also:


Attachments
Patch (31.45 KB, patch)
2020-01-09 19:24 PST, Peng Liu
no flags Details | Formatted Diff | Diff
WIP patch (90.09 KB, patch)
2020-01-11 00:54 PST, Peng Liu
no flags Details | Formatted Diff | Diff
WIP patch (90.02 KB, patch)
2020-01-11 01:01 PST, Peng Liu
no flags Details | Formatted Diff | Diff
Patch (51.51 KB, patch)
2020-01-11 11:40 PST, Peng Liu
no flags Details | Formatted Diff | Diff
Patch (update changelogs and fix unified build failures) (52.17 KB, patch)
2020-01-11 14:04 PST, Peng Liu
youennf: review+
Details | Formatted Diff | Diff
Patch for landing (52.82 KB, patch)
2020-01-13 15:33 PST, Peng Liu
no flags Details | Formatted Diff | Diff
Rebased patch (52.67 KB, patch)
2020-01-13 17:20 PST, Peng Liu
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Rebased patch for landing (53.81 KB, patch)
2020-01-14 11:40 PST, Peng Liu
no flags Details | Formatted Diff | Diff
Rebased patch for landing (48.27 KB, patch)
2020-01-14 15:20 PST, Peng Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peng Liu 2020-01-09 17:26:25 PST
[Media in GPU process] Implement the remote video layer support
Comment 1 Peng Liu 2020-01-09 19:24:51 PST
Created attachment 387309 [details]
Patch
Comment 2 youenn fablet 2020-01-10 03:23:01 PST
Comment on attachment 387309 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [Media in GPU process] Implement the remote video layer support

There are some Mac-wk2 test failures that might need to be investigated.

> Source/WebCore/platform/graphics/cocoa/WebCoreCALayerExtras.mm:91
> +CALayer* CreateWebRenderLayer(uint32_t contextID)

WebKit defines a LayerHostingContextID type, maybe we should type it?
Maybe we could use an ObjectIdentifier.

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:62
> +    m_layerHostingContext = LayerHostingContext::createForExternalHostingProcess();

Is it expensive to create?
Should we lazily initialise it? In prepareForPlayBack?

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:376
> +    m_player->setSize(m_videoSize);

Why do we need to set again m_videoSize? Is m_player losing this information somehow?

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:31
> +#include "LayerHostingContext.h"

We can probably forward declare LayerHostingContext instead.

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:74
> +    void seekWithTolerance(MediaTime&&, MediaTime&&, MediaTime&&);

The names seem useful since we have 3 values with the same type.

> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:103
> +        m_configuration = configuration;

WTFMove()
Comment 3 Simon Fraser (smfr) 2020-01-10 11:06:10 PST
Comment on attachment 387309 [details]
Patch

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

> Source/WebCore/platform/graphics/cocoa/CreateWebRenderLayer.h:30
> +WEBCORE_EXPORT CALayer* CreateWebRenderLayer(uint32_t contextID);

This file has a weird name; we don't usually make files to expose a single function. Also functions should not have an initial capital letter. Also, "Create" in Objective-C means that the function returns a retained object, but that's not the case here.

Maybe make a C++ class called WebRenderLayerFactory and give it a single static method.

> Source/WebCore/platform/graphics/cocoa/WebCoreCALayerExtras.mm:93
> +    return [CALayer _web_renderLayerWithContextID: contextID];

No space after :

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:62
> +    void prepareForPlayback(bool, WebCore::MediaPlayerEnums::Preload, bool, bool, CompletionHandler<void(uint32_t contextId)>&&);

Keep the parameter names.
Comment 4 Wenson Hsieh 2020-01-10 11:21:43 PST
Comment on attachment 387309 [details]
Patch

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

> Source/WebKit/Platform/mac/LayerHostingContext.h:68
> +    CALayer* rootLayer() const;

Nit - it should be "CALayer *" instead of "CALayer*", since CALayer is an Objective-C class (the style guidelines are a bit vague, but I think this is generally the rule we follow in WebKit).
Comment 5 Tim Horton 2020-01-10 11:30:11 PST
Comment on attachment 387309 [details]
Patch

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

>> Source/WebCore/platform/graphics/cocoa/CreateWebRenderLayer.h:30
>> +WEBCORE_EXPORT CALayer* CreateWebRenderLayer(uint32_t contextID);
> 
> This file has a weird name; we don't usually make files to expose a single function. Also functions should not have an initial capital letter. Also, "Create" in Objective-C means that the function returns a retained object, but that's not the case here.
> 
> Maybe make a C++ class called WebRenderLayerFactory and give it a single static method.

"WebRenderLayer" is not a great name either given what RenderLayer is. I think this perhaps goes somewhere like PlatformCALayer (maybe not exactly there since it returns a raw layer) as a static method, and is called something like createPlatformLayerForHostingContext(LayerHostingContextID contextID)
Comment 6 Peng Liu 2020-01-11 00:54:31 PST
Created attachment 387428 [details]
WIP patch
Comment 7 Peng Liu 2020-01-11 01:01:34 PST
Created attachment 387429 [details]
WIP patch
Comment 8 Peng Liu 2020-01-11 11:29:49 PST
Comment on attachment 387309 [details]
Patch

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

>> Source/WebCore/ChangeLog:3
>> +        [Media in GPU process] Implement the remote video layer support
> 
> There are some Mac-wk2 test failures that might need to be investigated.

Yes. I debugged the failures and found they are related to my implementation of MediaPlayerPrivateAVFoundationObjC::setSize(). I will remove MediaPlayerPrivateAVFoundationObjC::setSize(). The reasons are described in a comment below.

>>> Source/WebCore/platform/graphics/cocoa/CreateWebRenderLayer.h:30
>>> +WEBCORE_EXPORT CALayer* CreateWebRenderLayer(uint32_t contextID);
>> 
>> This file has a weird name; we don't usually make files to expose a single function. Also functions should not have an initial capital letter. Also, "Create" in Objective-C means that the function returns a retained object, but that's not the case here.
>> 
>> Maybe make a C++ class called WebRenderLayerFactory and give it a single static method.
> 
> "WebRenderLayer" is not a great name either given what RenderLayer is. I think this perhaps goes somewhere like PlatformCALayer (maybe not exactly there since it returns a raw layer) as a static method, and is called something like createPlatformLayerForHostingContext(LayerHostingContextID contextID)

Good suggestions! I changed the function name to be createPlatformLayerForHostingContext() and put it in LayerHostingContext as a static function.

>> Source/WebCore/platform/graphics/cocoa/WebCoreCALayerExtras.mm:91
>> +CALayer* CreateWebRenderLayer(uint32_t contextID)
> 
> WebKit defines a LayerHostingContextID type, maybe we should type it?
> Maybe we could use an ObjectIdentifier.

Fixed.

>> Source/WebCore/platform/graphics/cocoa/WebCoreCALayerExtras.mm:93
>> +    return [CALayer _web_renderLayerWithContextID: contextID];
> 
> No space after :

Fixed.

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:62
>> +    m_layerHostingContext = LayerHostingContext::createForExternalHostingProcess();
> 
> Is it expensive to create?
> Should we lazily initialise it? In prepareForPlayBack?

Moved to prepareForPlayback.

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:376
>> +    m_player->setSize(m_videoSize);
> 
> Why do we need to set again m_videoSize? Is m_player losing this information somehow?

When the setSize() function is called, the video layer might not be ready yet, so we need to save the size and use it when the video layer is ready. However, I realized that is not a good idea. The MediaPlayerPrivateAVFoundationObjc::setSize() is an empty function when the GPU Process is not enabled. We had better keep that behavior, so that we do not need to change the behavior of RenderVideo for the "media in GPU" feature. In addition, providing an empty setSize() function in MediaPlayerPrivateRemote can get rid of the meaningless XPC messages.

In the new patch, the setSize() function in RemoteMediaPlayerProxy and also MediaPlayerPrivateAVFoundationObjC are removed.
Also, the new patch implements RemoteMediaPlayerProxy::mediaPlayerContentBoxRect() to provide location and size information for the WebVideoContainerLayer when it  creates a video layer.

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:31
>> +#include "LayerHostingContext.h"
> 
> We can probably forward declare LayerHostingContext instead.

Fixed.

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:62
>> +    void prepareForPlayback(bool, WebCore::MediaPlayerEnums::Preload, bool, bool, CompletionHandler<void(uint32_t contextId)>&&);
> 
> Keep the parameter names.

Fixed.

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:74
>> +    void seekWithTolerance(MediaTime&&, MediaTime&&, MediaTime&&);
> 
> The names seem useful since we have 3 values with the same type.

Fixed.

>> Source/WebKit/Platform/mac/LayerHostingContext.h:68
>> +    CALayer* rootLayer() const;
> 
> Nit - it should be "CALayer *" instead of "CALayer*", since CALayer is an Objective-C class (the style guidelines are a bit vague, but I think this is generally the rule we follow in WebKit).

Got it, fixed.

>> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:103
>> +        m_configuration = configuration;
> 
> WTFMove()

Fixed.
Comment 9 Peng Liu 2020-01-11 11:40:05 PST
Created attachment 387438 [details]
Patch
Comment 10 Peng Liu 2020-01-11 11:48:48 PST
Comment on attachment 387438 [details]
Patch

With this patch, the video element can render video, but the size and location information may be incorrect because the RenderVideo is not able to manage the sublayers of WebVideoContainerLayer through the WebVideoHostingLayer. This issue will be fixed in webkit.org/b/206132.
Comment 11 Radar WebKit Bug Importer 2020-01-11 11:49:27 PST
<rdar://problem/58505616>
Comment 12 Peng Liu 2020-01-11 14:04:50 PST
Created attachment 387446 [details]
Patch (update changelogs and fix unified build failures)
Comment 13 youenn fablet 2020-01-13 01:26:20 PST
Comment on attachment 387446 [details]
Patch (update changelogs and fix unified build failures)

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

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:36
> +#import "CoreVideoSoftLink.h"

We usually add a line between SoftLink headers and others.

> Source/WebCore/platform/graphics/gpu/GPUSwapChainDescriptor.h:31
> +#include "GPUErrorScopes.h"

If it is a unified build issue, we usually fix this in cpp files, not. h files.

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.cpp:201
> +void RemoteMediaPlayerManagerProxy::prepareForPlayback(MediaPlayerPrivateRemoteIdentifier id, bool privateMode, WebCore::MediaPlayerEnums::Preload preload, bool preservesPitch, bool prepareForRendering, WebCore::LayoutPoint&& layoutPoint, WebCore::LayoutSize&& layoutSize, float videoContentScale, CompletionHandler<void(LayerHostingContextID contextId)>&& completionHandler)

Probably do not need to have &&, we could probably pass them by value.

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.cpp:203
>      if (auto player = m_proxies.get(id))

It would be better to call completionHandler even if there is no player.
Maybe contextId should be an Optional<>?

Maybe ASSERT(m_proxies.contains(id));

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.h:31
> +#include "LayerHostingContext.h"

Forward declare LayerHostingContextID?

> Source/WebKit/Platform/cocoa/LayerHostingContext.h:28
> +#include "LayerTreeContext.h"

Do we need this one?

> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:96
> +        m_layerHostingContextId = contextId;

m_layerHostingContextId does not seem used anywhere.

> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:97
> +        m_videoLayer = LayerHostingContext::createPlatformLayerForHostingContext(contextId);

contextId here can be 0, is it ok.
If we are using an ObjectIdentifier, we are sure this will not be 0.

> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:303
> +    uint32_t m_layerHostingContextId  { 0 };

Can we use an ObjectIdentifier<>?

> Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:308
> +    if (auto player = m_players.get(id))

s/auto/auto&/?
Comment 14 Simon Fraser (smfr) 2020-01-13 10:58:42 PST
Comment on attachment 387446 [details]
Patch (update changelogs and fix unified build failures)

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

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.cpp:201
>> +void RemoteMediaPlayerManagerProxy::prepareForPlayback(MediaPlayerPrivateRemoteIdentifier id, bool privateMode, WebCore::MediaPlayerEnums::Preload preload, bool preservesPitch, bool prepareForRendering, WebCore::LayoutPoint&& layoutPoint, WebCore::LayoutSize&& layoutSize, float videoContentScale, CompletionHandler<void(LayerHostingContextID contextId)>&& completionHandler)
> 
> Probably do not need to have &&, we could probably pass them by value.

Also if you're sending point and size, why not just send the rect?

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.messages.in:38
> +    PrepareForPlayback(WebKit::MediaPlayerPrivateRemoteIdentifier id, bool privateMode, enum:uint8_t WebCore::MediaPlayerEnums::Preload preload, bool preservesPitch, bool prepareForRendering, WebCore::LayoutPoint layoutPoint, WebCore::LayoutSize layoutSize, float videoContentScale) -> (WebKit::LayerHostingContextID layerHostingContextId) Async

Don't use 'id' (it's a reserved word in Objective-C).

> Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:306
> +void RemoteMediaPlayerManager::firstVideoFrameAvailable(WebKit::MediaPlayerPrivateRemoteIdentifier id)

Don't use 'id' as a parameter or variable name.
Comment 15 Peng Liu 2020-01-13 15:23:11 PST
Comment on attachment 387446 [details]
Patch (update changelogs and fix unified build failures)

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

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:36
>> +#import "CoreVideoSoftLink.h"
> 
> We usually add a line between SoftLink headers and others.

Fixed.

>> Source/WebCore/platform/graphics/gpu/GPUSwapChainDescriptor.h:31
>> +#include "GPUErrorScopes.h"
> 
> If it is a unified build issue, we usually fix this in cpp files, not. h files.

This change turns out to be unnecessary, so I removed it.

>>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.cpp:201
>>> +void RemoteMediaPlayerManagerProxy::prepareForPlayback(MediaPlayerPrivateRemoteIdentifier id, bool privateMode, WebCore::MediaPlayerEnums::Preload preload, bool preservesPitch, bool prepareForRendering, WebCore::LayoutPoint&& layoutPoint, WebCore::LayoutSize&& layoutSize, float videoContentScale, CompletionHandler<void(LayerHostingContextID contextId)>&& completionHandler)
>> 
>> Probably do not need to have &&, we could probably pass them by value.
> 
> Also if you're sending point and size, why not just send the rect?

There is no encode() or decode() for LayoutRect, so I used LayoutPoint and LayoutSize() instead. I will implement the encode() and decode() for LayoutRect in the updated patch and use LayoutRect (pass by value) as the function parameter.

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.cpp:203
>>      if (auto player = m_proxies.get(id))
> 
> It would be better to call completionHandler even if there is no player.
> Maybe contextId should be an Optional<>?
> 
> Maybe ASSERT(m_proxies.contains(id));

This is a good idea. I changed the contextId to be an Optional<>, and call the completionHandler witthout a contextId when there is no player.

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.h:31
>> +#include "LayerHostingContext.h"
> 
> Forward declare LayerHostingContextID?

Fixed.

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.messages.in:38
>> +    PrepareForPlayback(WebKit::MediaPlayerPrivateRemoteIdentifier id, bool privateMode, enum:uint8_t WebCore::MediaPlayerEnums::Preload preload, bool preservesPitch, bool prepareForRendering, WebCore::LayoutPoint layoutPoint, WebCore::LayoutSize layoutSize, float videoContentScale) -> (WebKit::LayerHostingContextID layerHostingContextId) Async
> 
> Don't use 'id' (it's a reserved word in Objective-C).

I see. Will fix it in webkit.org/b/206189.

>> Source/WebKit/Platform/cocoa/LayerHostingContext.h:28
>> +#include "LayerTreeContext.h"
> 
> Do we need this one?

Yes, it defines LayerHostingMode.

I replaced it with a forward declaration. And I have to include LayerTreeContext.h in PluginControllerProxyMac.mm and NetscapePlugin.cpp because of the change.

>> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:96
>> +        m_layerHostingContextId = contextId;
> 
> m_layerHostingContextId does not seem used anywhere.

Right, it does not need to be a member variable.

>> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:97
>> +        m_videoLayer = LayerHostingContext::createPlatformLayerForHostingContext(contextId);
> 
> contextId here can be 0, is it ok.
> If we are using an ObjectIdentifier, we are sure this will not be 0.

After removing m_layerHostingContextId, we will use the valid value return from the GPU Process.

>> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h:303
>> +    uint32_t m_layerHostingContextId  { 0 };
> 
> Can we use an ObjectIdentifier<>?

Yes, we can. But I think keeping the data type consistent with other places using LayerHostingContextID is a good idea.

>> Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:306
>> +void RemoteMediaPlayerManager::firstVideoFrameAvailable(WebKit::MediaPlayerPrivateRemoteIdentifier id)
> 
> Don't use 'id' as a parameter or variable name.

Will fix it in webkit.org/b/206189.

>> Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp:308
>> +    if (auto player = m_players.get(id))
> 
> s/auto/auto&/?

Changed to const auto&.
Comment 16 Peng Liu 2020-01-13 15:33:11 PST
Created attachment 387579 [details]
Patch for landing
Comment 17 Peng Liu 2020-01-13 17:20:24 PST
Created attachment 387597 [details]
Rebased patch
Comment 18 WebKit Commit Bot 2020-01-14 01:02:24 PST
Comment on attachment 387597 [details]
Rebased patch

Rejecting attachment 387597 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
DENABLE_INSPECTOR_TELEMETRY -DENABLE_INTERSECTION_OBSERVER -DENABLE_INTL -DENABLE_LAYOUT_FORMATTING_CONTEXT -DENABLE_LEGACY_CSS_VENDOR_PREFIXES -DENABLE_LEGACY_CUSTOM_PROTOCOL_MANAGER -DENABLE_LEGACY_ENCRYPTED_MEDIA -DENABLE_MATHML -DENABLE_MEDIA_CONTROLS_SCRIPT -DENABLE_MEDIA_SOURCE -DENABLE_MEDIA_STREAM -DENABLE_MEMORY_SAMPLER -DENABLE_METER_ELEMENT -DENABLE_MOUSE_CURSOR_SCALE -DENABLE_NETWORK_CACHE_SPECULATIVE_REVALIDATION -DENABLE_NOTIFICATIONS -DENABLE_PAYMENT_REQUEST -DENABLE_PDFKIT_PLUGIN -DENABLE_POINTER_EVENTS -DENABLE_POINTER_LOCK -DENABLE_PUBLIC_SUFFIX_LIST -DENABLE_REMOTE_INSPECTOR -DENABLE_RESIZE_OBSERVER -DENABLE_RESOURCE_LOAD_STATISTICS -DENABLE_RESOURCE_USAGE -DENABLE_RUBBER_BANDING -DENABLE_SANDBOX_EXTENSIONS -DENABLE_SERVER_PRECONNECT -DENABLE_SERVICE_CONTROLS -DENABLE_SERVICE_WORKER -DENABLE_SHAREABLE_RESOURCE -DENABLE_SPEECH_SYNTHESIS -DENABLE_STREAMS_API -DENABLE_SVG_FONTS -DENABLE_TELEPHONE_NUMBER_DETECTION -DENABLE_TEXT_AUTOSIZING -DENABLE_USERSELECT_ALL -DENABLE_USER_MESSAGE_HANDLERS -DENABLE_VARIATION_FONTS -DENABLE_VIDEO -DENABLE_VIDEO_PRESENTATION_MODE -DENABLE_VIDEO_TRACK -DENABLE_VIDEO_USES_ELEMENT_FULLSCREEN -DENABLE_WEBDRIVER_MOUSE_INTERACTIONS -DENABLE_WEBDRIVER_KEYBOARD_INTERACTIONS -DENABLE_WEBGL -DENABLE_WEBGL2 -DENABLE_WEBGPU -DENABLE_WEB_AUDIO -DENABLE_WEB_AUTHN -DENABLE_WEB_CRYPTO -DENABLE_WEB_RTC -DENABLE_WIRELESS_PLAYBACK_TARGET -DENABLE_XSLT -DHAVE_CORE_PREDICTION -DU_HIDE_DEPRECATED_API -DU_DISABLE_RENAMING=1 -DU_SHOW_CPLUSPLUS_API=0 -DFRAMEWORK_NAME=WebKit -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -fasm-blocks -fstrict-aliasing -Wdeprecated-declarations -Winvalid-offsetof -mmacosx-version-min=10.14 -g -fvisibility=hidden -fvisibility-inlines-hidden -fno-threadsafe-statics -Wno-sign-conversion -Winfinite-recursion -Wmove -Wcomma -Wblock-capture-autoreleasing -Wstrict-prototypes -Wrange-loop-analysis -Wno-semicolon-before-method-body -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/WebKit-generated-files.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/WebKit-own-target-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/WebKit-all-target-headers.hmap -iquote /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/WebKit-project-headers.hmap -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebCore.framework/PrivateHeaders/ForwardingHeaders -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2 -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/WebKitAdditions -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/local/include/WebKitAdditions -I/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/webrtc -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/local/include/webrtc -I/Volumes/Data/EWS/WebKit/Source/WebKit -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/DerivedSources-normal/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/DerivedSources/x86_64 -I/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/DerivedSources -Wall -Wextra -Wcast-qual -Wchar-subscripts -Wextra-tokens -Wformat-security -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wno-unused-parameter -Wpacked -Wpointer-arith -Wredundant-decls -Wundef -Wwrite-strings -Wexit-time-destructors -Wglobal-constructors -Wtautological-compare -Wimplicit-fallthrough -F/Volumes/Data/EWS/WebKit/WebKitBuild/Release -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/PrivateFrameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks -iframework /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/PrivateFrameworks -isystem /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/System.framework/PrivateHeaders -include /Volumes/Data/EWS/WebKit/WebKitBuild/PrecompiledHeaders/WebKit2Prefix-fznpdzohnkiztteqyhyuitpillqx/WebKit2Prefix.h -MMD -MT dependencies -MF /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/UnifiedSource9.d --serialize-diagnostics /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/UnifiedSource9.dia -c /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/unified-sources/UnifiedSource9.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebKit.build/Release/WebKit.build/Objects-normal/x86_64/UnifiedSource9.o

Full output: https://webkit-queues.webkit.org/results/13303867
Comment 19 Peng Liu 2020-01-14 11:40:22 PST
Created attachment 387680 [details]
Rebased patch for landing
Comment 20 Peng Liu 2020-01-14 15:20:50 PST
Created attachment 387712 [details]
Rebased patch for landing
Comment 21 WebKit Commit Bot 2020-01-14 18:51:26 PST
Comment on attachment 387712 [details]
Rebased patch for landing

Clearing flags on attachment: 387712

Committed r254555: <https://trac.webkit.org/changeset/254555>