1314530 - [EME][Fennec] Instantiate MediaDrmCDMProxy and pass MediaPrefs::PDMAndroidRemoteCodecEnabled() to MediaCDMProxy to determine RemoteMediaDrmBridge or LocalMediaDrmBridge

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect, P2)

Description

[James Cheng[:JamesCheng]]

Once bug 1306196, bug 1306185, bug 1306572 got landed, we need to create the MediaDrmBrige instance by the preference to determine it is oop case or non-oop case.

Comment 1

[James Cheng[:JamesCheng]]

Attachment: Bug 1314530 Part1 - Remove IsInPrivateBrowsing() since we did not use it anymore.

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

Comment 2

[James Cheng[:JamesCheng]]

Attachment: Bug 1314530 Part2 - Add isRemote flag to MediaDrmProxy::Create function to determine if it is oop case.

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

Comment 3

[James Cheng[:JamesCheng]]

Attachment: Bug 1314530 Part3 - Instantiate MediaDrmCDMProxy if it is widevine on fennec.

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

Comment 4

[James Cheng[:JamesCheng]]

Try result attached:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a8d5dd05e00

Comment 5

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8808040 [details]
Bug 1314530 Part2 - Add isRemote flag to MediaDrmProxy::Create function to determine if it is oop case.

https://reviewboard.mozilla.org/r/90964/#review90720

::: mobile/android/base/java/org/mozilla/gecko/media/MediaDrmProxy.java:253
(Diff revision 1)
>  
>      @WrapForJNI(calledFrom = "gecko")
>      public static MediaDrmProxy create(String keySystem,
> -                                       Callbacks nativeCallbacks) {
> +                                       Callbacks nativeCallbacks,
> +                                       boolean isRemote) {
> +        // TODO: Bug 1307818 wiil implement the instantiate logic by the isRemote flag

1) s/wiil/will
2) (Maybe) rephrase it to 
TODO: Will implement {Local,Remote}MediaDrmBridge instantiation by '''isRemote''' flag in Bug XXX.

Comment 6

[James Cheng[:JamesCheng]]

Comment on attachment 8808040 [details]
Bug 1314530 Part2 - Add isRemote flag to MediaDrmProxy::Create function to determine if it is oop case.

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

Comment 7

[James Cheng[:JamesCheng]]

Comment on attachment 8808041 [details]
Bug 1314530 Part3 - Instantiate MediaDrmCDMProxy if it is widevine on fennec.

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

Comment 8

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

Comment on attachment 8808039 [details]
Bug 1314530 Part1 - Remove IsInPrivateBrowsing() since we did not use it anymore.

https://reviewboard.mozilla.org/r/90962/#review90970

Comment 9

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

Comment on attachment 8808040 [details]
Bug 1314530 Part2 - Add isRemote flag to MediaDrmProxy::Create function to determine if it is oop case.

https://reviewboard.mozilla.org/r/90964/#review90972

Comment 10

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

Comment on attachment 8808041 [details]
Bug 1314530 Part3 - Instantiate MediaDrmCDMProxy if it is widevine on fennec.

https://reviewboard.mozilla.org/r/90966/#review90974

::: dom/media/eme/MediaKeys.h:137
(Diff revision 2)
>    bool IsBoundToMediaElement() const;
>  
>  private:
>  
> +  // Instantiate CDMProxy instance.
> +  // It could be MediaDrmCDMProxy(Widevine on Fennec) or GMPCDMProxy(the rest).

s/GMPCDMProxy(the rest)/GMPCDMProxy (the rest)/

::: dom/media/eme/MediaKeys.cpp:344
(Diff revision 2)
> +                                 mConfig.mDistinctiveIdentifier == MediaKeysRequirement::Required,
> +                                 mConfig.mPersistentState == MediaKeysRequirement::Required);
> +  } else
> +#endif
> +  {
> +    proxy= new GMPCDMProxy(this,

s/proxy=/proxy =/

Comment 11

[James Cheng[:JamesCheng]]

Comment on attachment 8808041 [details]
Bug 1314530 Part3 - Instantiate MediaDrmCDMProxy if it is widevine on fennec.

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

Comment 12

[James Cheng[:JamesCheng]]

Thanks for your review!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3879ad77bbcf
Part1 - Remove IsInPrivateBrowsing() since we did not use it anymore. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/13f6bd96aa78
Part2 - Add isRemote flag to MediaDrmProxy::Create function to determine if it is oop case. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/971aaa1646ed
Part3 - Instantiate MediaDrmCDMProxy if it is widevine on fennec. r=cpearce

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/3879ad77bbcf
https://hg.mozilla.org/mozilla-central/rev/13f6bd96aa78
https://hg.mozilla.org/mozilla-central/rev/971aaa1646ed