WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
209712
REGRESSION (
r257867
): [GPUP] Use-after-move in RemoteCDMInstance::createSession()
https://bugs.webkit.org/show_bug.cgi?id=209712
Summary
REGRESSION (r257867): [GPUP] Use-after-move in RemoteCDMInstance::createSessi...
David Kilzer (:ddkilzer)
Reported
2020-03-28 19:44:03 PDT
Use-after-move in RemoteCDMInstance::createSession() of `id` parameter: RefPtr<WebCore::CDMInstanceSession> RemoteCDMInstance::createSession() { if (!m_factory) return nullptr; RemoteCDMInstanceSessionIdentifier id; m_factory->gpuProcessConnection().connection().sendSync(Messages::RemoteCDMInstanceProxy::CreateSession(), Messages::RemoteCDMInstanceProxy::CreateSession::Reply(id), m_identifier); if (!id) return nullptr; auto session = RemoteCDMInstanceSession::create(makeWeakPtr(m_factory.get()), WTFMove(id)); m_factory->addSession(id, session.copyRef()); return session; } This regressed in:
Bug 208090
: [GPUP] Implement Modern EME API in the GPU Process <
https://bugs.webkit.org/show_bug.cgi?id=208090
> <
rdar://problem/59955230
> <
https://trac.webkit.org/changeset/257867
>
Attachments
Patch v1
(2.41 KB, patch)
2020-03-28 19:51 PDT
,
David Kilzer (:ddkilzer)
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2020-03-28 19:44:25 PDT
<
rdar://problem/61018795
>
David Kilzer (:ddkilzer)
Comment 2
2020-03-28 19:51:20 PDT
Created
attachment 394852
[details]
Patch v1
Darin Adler
Comment 3
2020-03-28 19:52:35 PDT
Comment on
attachment 394852
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=394852&action=review
> Source/WebKit/WebProcess/GPU/media/RemoteCDMInstance.cpp:94 > auto session = RemoteCDMInstanceSession::create(makeWeakPtr(m_factory.get()), WTFMove(id)); > - m_factory->addSession(id, session.copyRef()); > + m_factory->addSession(session->identifier(), session.copyRef());
How about just removing the WTFMove instead?
> Source/WebKit/WebProcess/GPU/media/RemoteCDMInstanceSession.h:46 > + RemoteCDMInstanceSessionIdentifier identifier() const { return m_identifier; }
Seems like this is pretty far to go just to fix the use after move.
David Kilzer (:ddkilzer)
Comment 4
2020-03-28 19:54:55 PDT
Comment on
attachment 394852
[details]
Patch v1 View in context:
https://bugs.webkit.org/attachment.cgi?id=394852&action=review
>> Source/WebKit/WebProcess/GPU/media/RemoteCDMInstanceSession.h:46 >> + RemoteCDMInstanceSessionIdentifier identifier() const { return m_identifier; } > > Seems like this is pretty far to go just to fix the use after move.
Adding this getter makes it possible for RemoteLegacyCDMFactory::addSession() not to even need the RemoteCDMInstanceSessionIdentifier argument as it can get it from the session object passed in. I was focusing on making the smallest change possible to fix the use-after-move, but should I go ahead and make that change as well?
Darin Adler
Comment 5
2020-03-28 19:56:32 PDT
Yes, keep this patch as small as possible. You can do other changes in another patch that is not fixing an obvious problem.
EWS
Comment 6
2020-03-29 01:52:44 PDT
Committed
r259176
: <
https://trac.webkit.org/changeset/259176
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 394852
[details]
.
David Kilzer (:ddkilzer)
Comment 7
2020-04-04 10:01:05 PDT
(In reply to Darin Adler from
comment #5
)
> Yes, keep this patch as small as possible. You can do other changes in > another patch that is not fixing an obvious problem.
Bug 210006
: [GPUP] Remove unnecessary RemoteCDMInstanceSessionIdentifier argument from RemoteCDMFactory::addSession()
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug