Bug 208402 - [GPUP] Implement Legacy EME API in the GPU Process
Summary: [GPUP] Implement Legacy EME API in the GPU Process
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-28 17:15 PST by Jer Noble
Modified: 2020-03-10 15:06 PDT (History)
13 users (show)

See Also:


Attachments
Part 1: WebCore (22.93 KB, patch)
2020-02-28 17:26 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Part 2: WebKit (119.62 KB, patch)
2020-02-28 17:30 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Part 1: WebCore (22.93 KB, patch)
2020-02-29 19:39 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Part 2: WebKit (117.74 KB, patch)
2020-02-29 19:40 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Part 1: WebCore (15.91 KB, patch)
2020-03-05 23:00 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Part 2: WebKit (117.37 KB, patch)
2020-03-05 23:13 PST, Jer Noble
eric.carlson: review+
Details | Formatted Diff | Diff
Part 1: WebCore (16.43 KB, patch)
2020-03-06 16:28 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (118.23 KB, patch)
2020-03-08 12:03 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (118.23 KB, patch)
2020-03-08 12:21 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (118.23 KB, patch)
2020-03-08 12:24 PDT, Jer Noble
no flags Details | Formatted Diff | Diff
Patch for landing (118.09 KB, patch)
2020-03-08 12:33 PDT, Jer Noble
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Follow up fix (1.36 KB, patch)
2020-03-09 11:31 PDT, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2020-02-28 17:15:48 PST
[GPUP] Implement Legacy EME API in the GPU Process
Comment 1 Jer Noble 2020-02-28 17:26:18 PST
Created attachment 392036 [details]
Part 1: WebCore
Comment 2 Jer Noble 2020-02-28 17:30:52 PST
Created attachment 392038 [details]
Part 2: WebKit
Comment 3 Jer Noble 2020-02-29 19:39:26 PST
Created attachment 392087 [details]
Part 1: WebCore
Comment 4 Jer Noble 2020-02-29 19:40:02 PST
Created attachment 392088 [details]
Part 2: WebKit
Comment 5 Jer Noble 2020-03-05 23:00:33 PST
Created attachment 392683 [details]
Part 1: WebCore
Comment 6 Jer Noble 2020-03-05 23:13:40 PST
Created attachment 392684 [details]
Part 2: WebKit
Comment 7 Jer Noble 2020-03-06 16:28:29 PST
Created attachment 392800 [details]
Part 1: WebCore
Comment 8 Radar WebKit Bug Importer 2020-03-06 18:02:11 PST
<rdar://problem/60178488>
Comment 9 WebKit Commit Bot 2020-03-07 21:13:15 PST
Comment on attachment 392800 [details]
Part 1: WebCore

Clearing flags on attachment: 392800

Committed r258100: <https://trac.webkit.org/changeset/258100>
Comment 10 Eric Carlson 2020-03-08 11:37:13 PDT
Comment on attachment 392684 [details]
Part 2: WebKit

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

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:82
> +    gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem(keySystem, WTF::nullopt), Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem::Reply(supported), { });

It would be good to cache results so we only need to make one sync IPC per key system

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:89
> +    gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem(keySystem, mimeType), Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem::Reply(supported), { });

Ditto for key system + MIME type

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:141
> +
> +

Nit: two blanks

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.cpp:72
> +    return { SharedBuffer::create((const char*)array->data(), array->byteLength()) };

Is the cast necessary (you don't have one in convertToOptionalDataReference)?

> Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.cpp:131
> +    m_factory->gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyCDMSessionProxy::CachedKeyForKeyID(keyId), Messages::RemoteLegacyCDMSessionProxy::CachedKeyForKeyID::Reply(ipcKey), m_identifier);

Can these be cached here so we only need to make one sync IPC per cached key ID?
Comment 11 Jer Noble 2020-03-08 11:49:39 PDT
(In reply to Eric Carlson from comment #10)
> Comment on attachment 392684 [details]
> Part 2: WebKit
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=392684&action=review
> 
> > Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:82
> > +    gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem(keySystem, WTF::nullopt), Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem::Reply(supported), { });
> 
> It would be good to cache results so we only need to make one sync IPC per
> key system
> 
> > Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:89
> > +    gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem(keySystem, mimeType), Messages::RemoteLegacyCDMFactoryProxy::SupportsKeySystem::Reply(supported), { });
> 
> Ditto for key system + MIME type

Okay, will do for both.

> > Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp:141
> > +
> > +
> 
> Nit: two blanks

Removed.

> > Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.cpp:72
> > +    return { SharedBuffer::create((const char*)array->data(), array->byteLength()) };
> 
> Is the cast necessary (you don't have one in convertToOptionalDataReference)?

Looks like it's not necessary. Removed.

> > Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.cpp:131
> > +    m_factory->gpuProcessConnection().connection().sendSync(Messages::RemoteLegacyCDMSessionProxy::CachedKeyForKeyID(keyId), Messages::RemoteLegacyCDMSessionProxy::CachedKeyForKeyID::Reply(ipcKey), m_identifier);
> 
> Can these be cached here so we only need to make one sync IPC per cached key
> ID?

This one is harder than the other two, since it's expected that the cached key will change. We'd need to add invalidation messages when the cached key changes, but I guess we could. Let me try.
Comment 12 Jer Noble 2020-03-08 12:03:45 PDT
Created attachment 392967 [details]
Patch for landing
Comment 13 Jer Noble 2020-03-08 12:21:21 PDT
Created attachment 392970 [details]
Patch for landing
Comment 14 Jer Noble 2020-03-08 12:24:23 PDT
Created attachment 392971 [details]
Patch for landing
Comment 15 Jer Noble 2020-03-08 12:33:20 PDT
Created attachment 392974 [details]
Patch for landing
Comment 16 Jer Noble 2020-03-08 17:34:53 PDT
Committed r258122: <https://trac.webkit.org/changeset/258122>
Comment 17 WebKit Commit Bot 2020-03-08 17:35:51 PDT
The commit-queue encountered the following flaky tests while processing attachment 392974 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 160571 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 18 Jer Noble 2020-03-09 11:31:17 PDT
Reopening to attach new patch.
Comment 19 Jer Noble 2020-03-09 11:31:18 PDT
Created attachment 393057 [details]
Follow up fix
Comment 20 WebKit Commit Bot 2020-03-09 11:32:31 PDT
Comment on attachment 392974 [details]
Patch for landing

Rejecting attachment 392974 [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', 'apply-attachment', '--no-update', '--non-interactive', 392974, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 5000 characters of output:
hing file Source/WebKit/GPUProcess/media/RemoteLegacyCDMSessionProxy.cpp
patching file Source/WebKit/GPUProcess/media/RemoteLegacyCDMSessionProxy.h
patching file Source/WebKit/GPUProcess/media/RemoteLegacyCDMSessionProxy.messages.in
patching file Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.cpp
Hunk #1 succeeded at 212 with fuzz 1 (offset 8 lines).
patching file Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.h
Hunk #1 succeeded at 70 with fuzz 2 (offset 2 lines).
patching file Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp
Hunk #1 FAILED at 33.
Hunk #2 FAILED at 525.
Hunk #3 FAILED at 729.
3 out of 3 hunks FAILED -- saving rejects to file Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp.rej
patching file Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h
Hunk #1 FAILED at 30.
Hunk #2 FAILED at 145.
Hunk #3 succeeded at 181 with fuzz 2 (offset 4 lines).
Hunk #4 FAILED at 301.
3 out of 4 hunks FAILED -- saving rejects to file Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h.rej
patching file Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.messages.in
Hunk #1 FAILED at 80.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.messages.in.rej
patching file Source/WebKit/GPUProcess/media/RemoteTextTrackProxy.cpp
Hunk #1 FAILED at 29.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/GPUProcess/media/RemoteTextTrackProxy.cpp.rej
patching file Source/WebKit/GPUProcess/media/RemoteTextTrackProxy.h
Hunk #1 FAILED at 32.
Hunk #2 FAILED at 57.
Hunk #3 FAILED at 74.
Hunk #4 FAILED at 88.
4 out of 4 hunks FAILED -- saving rejects to file Source/WebKit/GPUProcess/media/RemoteTextTrackProxy.h.rej
patching file Source/WebKit/Scripts/webkit/messages.py
Hunk #1 FAILED at 238.
Hunk #2 FAILED at 586.
2 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/Scripts/webkit/messages.py.rej
patching file Source/WebKit/Sources.txt
Hunk #1 FAILED at 33.
Hunk #2 FAILED at 558.
2 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/Sources.txt.rej
patching file Source/WebKit/WebKit.xcodeproj/project.pbxproj
Hunk #1 FAILED at 1747.
Hunk #2 FAILED at 5144.
Hunk #3 FAILED at 5476.
Hunk #4 FAILED at 5534.
Hunk #5 FAILED at 9987.
Hunk #6 FAILED at 12665.
6 out of 6 hunks FAILED -- saving rejects to file Source/WebKit/WebKit.xcodeproj/project.pbxproj.rej
patching file Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp
Hunk #1 FAILED at 35.
Hunk #2 succeeded at 89 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 108 (offset 13 lines).
1 out of 3 hunks FAILED -- saving rejects to file Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp.rej
patching file Source/WebKit/WebProcess/GPU/GPUProcessConnection.h
Hunk #1 FAILED at 41.
Hunk #2 FAILED at 63.
2 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/WebProcess/GPU/GPUProcessConnection.h.rej
patching file Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp
Hunk #1 FAILED at 30.
Hunk #2 FAILED at 41.
Hunk #3 succeeded at 67 with fuzz 1 (offset 9 lines).
Hunk #4 FAILED at 928.
3 out of 4 hunks FAILED -- saving rejects to file Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp.rej
patching file Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h
Hunk #1 FAILED at 305.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.h.rej
patching file Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.messages.in
Hunk #1 succeeded at 82 with fuzz 1 (offset 4 lines).
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDM.cpp
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDM.h
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.cpp
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMFactory.h
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMIdentifier.h
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.cpp
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.h
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSession.messages.in
patching file Source/WebKit/WebProcess/GPU/media/RemoteLegacyCDMSessionIdentifier.h
patching file Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.cpp
Hunk #1 succeeded at 194 with fuzz 2 (offset 11 lines).
patching file Source/WebKit/WebProcess/GPU/media/RemoteMediaPlayerManager.h
Hunk #1 succeeded at 71 with fuzz 2 (offset 2 lines).
patching file Source/WebKit/WebProcess/WebPage/WebPage.cpp
Hunk #1 succeeded at 330 with fuzz 2 (offset 4 lines).
patching file Source/WebKit/WebProcess/WebProcess.cpp
Hunk #1 FAILED at 43.
Hunk #2 succeeded at 2034 with fuzz 2 (offset 8 lines).
1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/WebProcess/WebProcess.cpp.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/13337172
Comment 21 WebKit Commit Bot 2020-03-09 13:23:50 PDT
Comment on attachment 393057 [details]
Follow up fix

Clearing flags on attachment: 393057

Committed r258160: <https://trac.webkit.org/changeset/258160>