Last Comment Bug 767636 - Update nsIObjectLoadingContent interface for click-to-play
: Update nsIObjectLoadingContent interface for click-to-play
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: David Keeler [:keeler] (use needinfo?)
:
Mentors:
Depends on: 745030 783836
Blocks: click-to-play 776208
  Show dependency treegraph
 
Reported: 2012-06-22 17:13 PDT by John Schoenick [:johns]
Modified: 2012-09-04 13:34 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
pluginSupportState (12.26 KB, patch)
2012-08-02 11:57 PDT, David Keeler [:keeler] (use needinfo?)
no flags Details | Diff | Review
patch (13.73 KB, patch)
2012-08-08 17:08 PDT, David Keeler [:keeler] (use needinfo?)
jaas: feedback+
Details | Diff | Review
pluginFallbackType (13.94 KB, patch)
2012-08-15 10:36 PDT, David Keeler [:keeler] (use needinfo?)
jaas: review+
john: checkin+
Details | Diff | Review
proposed changes to fix bug 783836 (8.57 KB, patch)
2012-08-18 21:39 PDT, Yury Delendik (:yury)
no flags Details | Diff | Review
pluginFallbackType v2 (14.96 KB, patch)
2012-08-20 15:09 PDT, David Keeler [:keeler] (use needinfo?)
jaws: review+
john: checkin+
Details | Diff | Review

Description John Schoenick [:johns] 2012-06-22 17:13:10 PDT
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 [:bz] (Out June 25-July 6) 2012-06-22 18:06:16 PDT
Just watch out: web code can generally call into the methods on this interface.
Comment 2 David Keeler [:keeler] (use needinfo?) 2012-08-02 11:57:50 PDT
Created attachment 648420 [details] [diff] [review]
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 David Keeler [:keeler] (use needinfo?) 2012-08-08 17:08:18 PDT
Created attachment 650372 [details] [diff] [review]
patch
Comment 4 Josh Aas 2012-08-15 10:06:22 PDT
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 David Keeler [:keeler] (use needinfo?) 2012-08-15 10:36:15 PDT
Created attachment 652138 [details] [diff] [review]
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 David Keeler [:keeler] (use needinfo?) 2012-08-17 10:21:29 PDT
This ran green: https://tbpl.mozilla.org/?tree=Try&rev=15c4080ea5e1
Marking checkin-needed.
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-08-17 19:21:43 PDT
https://hg.mozilla.org/mozilla-central/rev/2b5b8baa5816
Comment 9 Ed Morley [:emorley] 2012-08-18 18:09:57 PDT
Backed out for bug 783836:
https://hg.mozilla.org/integration/mozilla-inbound/rev/175c0a74f744
Comment 10 Yury Delendik (:yury) 2012-08-18 21:39:23 PDT
Created attachment 653135 [details] [diff] [review]
proposed changes to fix bug 783836

Looks like the |plugin.QueryInterface(Ci.nsIObjectLoadingContent);| interface queries are missed in the tests.
Comment 11 David Keeler [:keeler] (use needinfo?) 2012-08-20 15:09:31 PDT
Created attachment 653542 [details] [diff] [review]
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 (Away 6/25-7/4) Jared Wein [:jaws] (reviews and needinfo disabled until back) 2012-08-20 16:18:11 PDT
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 David Keeler [:keeler] (use needinfo?) 2012-08-20 16:23:57 PDT
Let's see if this works better: https://tbpl.mozilla.org/?tree=Try&rev=6985b1f34ab2
Comment 14 John Schoenick [:johns] 2012-08-20 17:04:21 PDT
(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 David Keeler [:keeler] (use needinfo?) 2012-08-21 11:35:33 PDT
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] 2012-08-21 11:37:41 PDT
(In reply to David Keeler from comment #15)
> something weird is going on.

Or clobber vs non-clobber bustage :-)
Comment 18 Ryan VanderMeulen [:RyanVM] 2012-08-21 19:08:33 PDT
https://hg.mozilla.org/mozilla-central/rev/0ca713e1db47

Note You need to log in before you can comment on or make changes to this bug.