Bug 208699

Summary: [GPUP] Convert CDMFactory away from platformStrategies() and use WebProcess settings instead
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, commit-queue, eric.carlson, ews-watchlist, glenn, peng.liu6, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
youennf: review+
Patch for landing
commit-queue: commit-queue-
Patch for landing
jer.noble: commit-queue+
Patch for landing none

Description Jer Noble 2020-03-05 22:38:48 PST
[GPUP] Convert CDMFactory away from platformStrategies() and use WebProcess settings instead
Comment 1 Jer Noble 2020-03-05 22:43:35 PST
Created attachment 392682 [details]
Patch
Comment 2 youenn fablet 2020-03-06 05:35:48 PST
Comment on attachment 392682 [details]
Patch

r=me as long as bots are green.

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

> Source/WebKit/WebProcess/WebProcess.cpp:2011
> +    cdmFactories.clear();

This seems a bit odd to get the vector, clear it and then refill it and passing it as a mutable parameter.
I guess this might help for extensibility.

Could we just have something like :
static inline Vector<CDMFactory*> computeFactories()
{
   if (useGPUProcessForMedia)
     return ensureGPUProcessConnection().cdmFactory().computeFactories();
  return CDMFactory::computePlatformFactories();
}
...

    CDMFactory::registerFactories(computeFactories())

> Source/WebKit/WebProcess/WebProcess.cpp:2013
> +    if (useGPUProcessForMedia)

Probably missing a ENABLE(GPU_PROCESS)
Comment 4 Jer Noble 2020-03-06 15:09:39 PST
(In reply to Aakash Jain from comment #3)
> Please check
> https://ews-build.webkit.org/#/builders/8/builds/17625/steps/8/logs/stdio

Yes, that's what Youenn's comment about a missing ENABLE(GPU_PROCESS) guard addresses.
Comment 5 Jer Noble 2020-03-06 15:10:52 PST
(In reply to youenn fablet from comment #2)
> Comment on attachment 392682 [details]
> Patch
> 
> r=me as long as bots are green.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=392682&action=review
> 
> > Source/WebKit/WebProcess/WebProcess.cpp:2011
> > +    cdmFactories.clear();
> 
> This seems a bit odd to get the vector, clear it and then refill it and
> passing it as a mutable parameter.
> I guess this might help for extensibility.

I took a pass at this, but it requires a lot of re-architecture of port-specific code; it should be left as a clean-up task.
Comment 6 Jer Noble 2020-03-06 15:11:47 PST
Created attachment 392782 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2020-03-06 16:28:01 PST
ChangeLog entry in Source/WebKitLegacy/mac/ChangeLog contains OOPS!.
Comment 8 WebKit Commit Bot 2020-03-06 16:28:15 PST
Comment on attachment 392782 [details]
Patch for landing

Rejecting attachment 392782 [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-01', 'validate-changelog', '--check-oops', '--non-interactive', 392782, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebKitLegacy/mac/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/13335013
Comment 9 Jer Noble 2020-03-06 16:36:54 PST
Created attachment 392805 [details]
Patch for landing
Comment 10 Jer Noble 2020-03-06 16:37:44 PST
Created attachment 392806 [details]
Patch for landing
Comment 11 WebKit Commit Bot 2020-03-06 17:25:35 PST
Comment on attachment 392806 [details]
Patch for landing

Clearing flags on attachment: 392806

Committed r258040: <https://trac.webkit.org/changeset/258040>
Comment 12 Radar WebKit Bug Importer 2020-03-06 17:58:13 PST
<rdar://problem/60178373>