1339755 - Move GMPCDMProxy::DecryptJob out of GMPCDMProxy

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

This means it can be reused in other CDMProxies, specifically, my work to make a new Chromium CDMProxy in bug Bug 1315850.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1339755 - Move GMPCDMProxy::DecryptJob out of GMPCDMProxy.

This means it can be used in other CDMProxies, specifically the
upcoming Chromium CDM code in bug 1315850.

Review commit: https://reviewboard.mozilla.org/r/112668/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/112668/

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1339755 - Move DecryptJob id calculation into its constructor.

The job id is just a counter, so rather than have other users of DecryptJob
reimplement their own counter, we can push the id/counter code into DecryptJob
itself.

Review commit: https://reviewboard.mozilla.org/r/112670/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/112670/

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8837502 [details]
Bug 1339755 - Move GMPCDMProxy::DecryptJob out of GMPCDMProxy.

https://reviewboard.mozilla.org/r/112668/#review114098

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8837503 [details]
Bug 1339755 - Move DecryptJob id calculation into its constructor.

https://reviewboard.mozilla.org/r/112670/#review114102

::: dom/media/gmp/DecryptJob.cpp:13
(Diff revision 1)
>  namespace mozilla {
>  
> +uint32_t sDecryptJobInstanceCount = 0;
> +
> +DecryptJob::DecryptJob(MediaRawData* aSample)
> +  : mId(++sDecryptJobInstanceCount )

Is there a data race if DecryptJob is created on different theads concurrently?

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8837503 [details]
Bug 1339755 - Move DecryptJob id calculation into its constructor.

https://reviewboard.mozilla.org/r/112670/#review114102

> Is there a data race if DecryptJob is created on different theads concurrently?

Under current usage patterns, this is always accessed from the same thread. I can make it atomic to make it safe. I'm expecting to remove the ID soon.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8837502 [details]
Bug 1339755 - Move GMPCDMProxy::DecryptJob out of GMPCDMProxy.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/112668/diff/1-2/

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8837503 [details]
Bug 1339755 - Move DecryptJob id calculation into its constructor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/112670/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85641936e471
Move GMPCDMProxy::DecryptJob out of GMPCDMProxy. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/f3629503d405
Move DecryptJob id calculation into its constructor. r=jwwang

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/autoland/rev/c187196bff43 for build bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=77696264&repo=autoland

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8837503 [details]
Bug 1339755 - Move DecryptJob id calculation into its constructor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/112670/diff/2-3/

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8837503 [details]
Bug 1339755 - Move DecryptJob id calculation into its constructor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/112670/diff/3-4/

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8837503 [details]
Bug 1339755 - Move DecryptJob id calculation into its constructor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/112670/diff/4-5/

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8837502 [details]
Bug 1339755 - Move GMPCDMProxy::DecryptJob out of GMPCDMProxy.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/112668/diff/2-3/

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8837503 [details]
Bug 1339755 - Move DecryptJob id calculation into its constructor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/112670/diff/5-6/

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53680343b6a2
Move GMPCDMProxy::DecryptJob out of GMPCDMProxy. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/18975ff8fe82
Move DecryptJob id calculation into its constructor. r=jwwang

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/53680343b6a2
https://hg.mozilla.org/mozilla-central/rev/18975ff8fe82