Handle createCDM() when frame destroyed

Handle createCDM() when frame destroyed

createCDM() now uses WeakPtr<> so that it fails gracefully when the
frame is destroyed.

BUG=454278
TEST=fuzzed test doesn't crash

Committed: https://crrev.com/d17dc45807d6041ea9df322a4fd4ecefbfcd1664
Cr-Commit-Position: refs/heads/master@{#314417}

[jrummell, 2015-02-02 22:18:40]

jrummell@chromium.org changed reviewers:
+ sandersd@chromium.org, xhwang@chromium.org

[jrummell, 2015-02-02 22:18:40]

PTAL.

[xhwang, 2015-02-03 18:45:49]

Thanks for the fix! lgtm with some nits

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
File media/blink/webcontentdecryptionmoduleaccess_impl.cc (right):

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:5: #include
"webcontentdecryptionmoduleaccess_impl.h"
full path relative to src/

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:11: #include
"webencryptedmediaclient_impl.h"
full path relative to src/

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:16: static void
CreateCdm(base::WeakPtr<WebEncryptedMediaClientImpl> client,
Add include for weak ptr.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:19:
blink::WebContentDecryptionModuleResult result) {
Pass parameters by const-ref where possible?

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:22: if (client.get() ==
nullptr) {
WeakPtr is Testable, no need to use get() and compare to nullptr:

See:
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/weak_p...

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:37:
base::WeakPtr<WebEncryptedMediaClientImpl> client) {
ditto

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:46:
base::WeakPtr<WebEncryptedMediaClientImpl> client)
ditto

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
File media/blink/webcontentdecryptionmoduleaccess_impl.h (right):

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.h:32:
base::WeakPtr<WebEncryptedMediaClientImpl> client);
pass by const ref?

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.h:45:
base::WeakPtr<WebEncryptedMediaClientImpl> client);
ditto

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.h:52:
base::WeakPtr<WebEncryptedMediaClientImpl> client_;
Comment about why this needs to be a weak ptr?

https://codereview.chromium.org/886393003/diff/1/media/blink/webencryptedmedi...
File media/blink/webencryptedmediaclient_impl.h (right):

https://codereview.chromium.org/886393003/diff/1/media/blink/webencryptedmedi...
media/blink/webencryptedmediaclient_impl.h:37: blink::WebString key_system,
nit: We usually have key system before the security origin. You comment also
reflects that convention ;)

[jrummell, 2015-02-03 20:01:14]

Thanks for the review.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
File media/blink/webcontentdecryptionmoduleaccess_impl.cc (right):

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:5: #include
"webcontentdecryptionmoduleaccess_impl.h"
On 2015/02/03 18:45:48, xhwang wrote:
> full path relative to src/

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:11: #include
"webencryptedmediaclient_impl.h"
On 2015/02/03 18:45:49, xhwang wrote:
> full path relative to src/

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:16: static void
CreateCdm(base::WeakPtr<WebEncryptedMediaClientImpl> client,
On 2015/02/03 18:45:49, xhwang wrote:
> Add include for weak ptr.

Already in the .h file.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:19:
blink::WebContentDecryptionModuleResult result) {
On 2015/02/03 18:45:49, xhwang wrote:
> Pass parameters by const-ref where possible?

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:22: if (client.get() ==
nullptr) {
On 2015/02/03 18:45:48, xhwang wrote:
> WeakPtr is Testable, no need to use get() and compare to nullptr:
> 
> See:
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/weak_p...

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:37:
base::WeakPtr<WebEncryptedMediaClientImpl> client) {
On 2015/02/03 18:45:48, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.cc:46:
base::WeakPtr<WebEncryptedMediaClientImpl> client)
On 2015/02/03 18:45:49, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
File media/blink/webcontentdecryptionmoduleaccess_impl.h (right):

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.h:32:
base::WeakPtr<WebEncryptedMediaClientImpl> client);
On 2015/02/03 18:45:49, xhwang wrote:
> pass by const ref?

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.h:45:
base::WeakPtr<WebEncryptedMediaClientImpl> client);
On 2015/02/03 18:45:49, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmoduleaccess_impl.h:52:
base::WeakPtr<WebEncryptedMediaClientImpl> client_;
On 2015/02/03 18:45:49, xhwang wrote:
> Comment about why this needs to be a weak ptr?

Done.

https://codereview.chromium.org/886393003/diff/1/media/blink/webencryptedmedi...
File media/blink/webencryptedmediaclient_impl.h (right):

https://codereview.chromium.org/886393003/diff/1/media/blink/webencryptedmedi...
media/blink/webencryptedmediaclient_impl.h:37: blink::WebString key_system,
On 2015/02/03 18:45:49, xhwang wrote:
> nit: We usually have key system before the security origin. You comment also
> reflects that convention ;)

Done.

[jrummell, 2015-02-03 20:01:17]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2015-02-03 20:02:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/886393003/20001

[commit-bot: I haz the power, 2015-02-03 22:09:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-03 22:12:24]

Patchset 2 (id:??) landed as
https://crrev.com/d17dc45807d6041ea9df322a4fd4ecefbfcd1664
Cr-Commit-Position: refs/heads/master@{#314417}