767636 - Update nsIObjectLoadingContent interface for click-to-play

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[John Schoenick [:johns]]

With the addition of click-to-play, adding functions to nsIObjectLoadingContent to let extensions more completely control and inspect c2p behavior is desirable

Comment 1

[Boris Zbarsky [:bzbarsky]]

Just watch out: web code can generally call into the methods on this interface.

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: pluginSupportState

One thing that would be extremely useful for the in-browser ui would be to expose a readonly attribute that says what plugin support state nsObjectLoadingContent came up with (essentially make mFallbackReason readable).

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: patch

Comment 4

[Josh Aas]

Comment on attachment 650372 [details] [diff] [review]
patch

Review of attachment 650372 [details] [diff] [review]:
-----------------------------------------------------------------

Seems like a reasonable approach for extensions.

::: content/base/src/nsObjectLoadingContent.cpp
@@ +2441,5 @@
>    return NS_OK;
>  }
>  
> +NS_IMETHODIMP
> +nsObjectLoadingContent::GetPluginFallbackType(PRUint32* aPluginFallbackType)

You might want to add a chrome/content check here, and limit this to chrome callers.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: pluginFallbackType

Added code to limit this to chrome/extensions. I think we're ready to go ahead with this, so asking for review.

Comment 6

[Dana Keeler (she/her) (use needinfo) [:keeler]]

This ran green: https://tbpl.mozilla.org/?tree=Try&rev=15c4080ea5e1
Marking checkin-needed.

Comment 7

[John Schoenick [:johns]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b5b8baa5816

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2b5b8baa5816

Comment 9

[Ed Morley [:emorley]]

Backed out for bug 783836:
https://hg.mozilla.org/integration/mozilla-inbound/rev/175c0a74f744

Comment 10

[Yury Delendik (:yury)]

Attachment: proposed changes to fix bug 783836

Looks like the |plugin.QueryInterface(Ci.nsIObjectLoadingContent);| interface queries are missed in the tests.

Comment 11

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: pluginFallbackType v2

Thanks to Yury for pointing out the QueryInterface issue.
Also, I forgot to rev the uuid, so that was wrong.
Asking for review from Jared in case there's something else wrong he might catch.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 653542 [details] [diff] [review]
pluginFallbackType v2

Review of attachment 653542 [details] [diff] [review]:
-----------------------------------------------------------------

r=me for the test changes. ship it! :)

Comment 13

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Let's see if this works better: https://tbpl.mozilla.org/?tree=Try&rev=6985b1f34ab2

Comment 14

[John Schoenick [:johns]]

(In reply to David Keeler from comment #11)
> Also, I forgot to rev the uuid, so that was wrong.

(I actually rev'd it in the landing, but forgot to mention it):
https://hg.mozilla.org/mozilla-central/rev/2b5b8baa5816#l2.13

Comment 15

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Okay - this ran green on try. If it fails on inbound again, something weird is going on.
Marking checkin-needed.

Comment 16

[Ed Morley [:emorley]]

(In reply to David Keeler from comment #15)
> something weird is going on.

Or clobber vs non-clobber bustage :-)

Comment 17

[John Schoenick [:johns]]

pushed:
http://hg.mozilla.org/integration/mozilla-inbound/rev/0ca713e1db47

try:
https://tbpl.mozilla.org/?tree=Try&rev=6985b1f34ab2

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0ca713e1db47