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>
<rdar://problem/61018795>
Created attachment 394852 [details] Patch v1
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.
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?
Yes, keep this patch as small as possible. You can do other changes in another patch that is not fixing an obvious problem.
Committed r259176: <https://trac.webkit.org/changeset/259176> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394852 [details].
(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()