Last Comment Bug 758363 - Implement NPN_ReloadPlugins for out-of-process Plugins
: Implement NPN_ReloadPlugins for out-of-process Plugins
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86_64 Windows 7
: -- normal (vote)
: mozilla15
Assigned To: Josh Aas
:
Mentors:
Depends on:
Blocks: 767612
  Show dependency treegraph
 
Reported: 2012-05-24 13:51 PDT by Stephen A Pohl [:spohl]
Modified: 2012-06-24 09:13 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed
+
fixed


Attachments
fix v1.0 (2.72 KB, patch)
2012-05-24 15:04 PDT, Josh Aas
no flags Details | Diff | Splinter Review
fix v1.1 (2.72 KB, patch)
2012-05-24 16:32 PDT, Josh Aas
benjamin: review+
akeybl: approval‑mozilla‑beta+
akeybl: approval‑mozilla‑release+
Details | Diff | Splinter Review

Description Stephen A Pohl [:spohl] 2012-05-24 13:51:35 PDT
User Agent: Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; WOW64; Trident/5.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E)

Steps to reproduce:

As of version 11.2, Flash Player will attempt to update in the background even if it is loaded by Firefox. At runtime, Flash Player performs checks to see if the latest version is being used. If not, Flash Player will call NPN_ReloadPlugins(FALSE) to load the latest version for any future instances.


Actual results:

NPN_ReloadPlugins isn't implemented for out-of-process Plugins. See dom/plugins/ipc/PluginModuleChild.cpp:

void NP_CALLBACK
_reloadplugins(NPBool aReloadPages)
{
    PLUGIN_LOG_DEBUG_FUNCTION;
    ENSURE_PLUGIN_THREAD_VOID();
    NS_WARNING("Not yet implemented!");
}


Expected results:

NPN_ReloadPlugins should be implemented for out-of-process Plugins.
Comment 1 Josh Aas 2012-05-24 15:04:19 PDT
Created attachment 626984 [details] [diff] [review]
fix v1.0

This is entirely untested, ran out of time today. It might work, it might need a bit more work.
Comment 2 Josh Aas 2012-05-24 15:05:28 PDT
Note: We might want to consider making the IPC call async. I made it sync for the first try in case there were dependencies in js code expecting plugins to be properly reloaded after the call.
Comment 3 Josh Aas 2012-05-24 16:32:17 PDT
Created attachment 627020 [details] [diff] [review]
fix v1.1

This seems to work, just switched the IPC call to async in this revision of the patch. Can you verify that this works for you, Stephen?
Comment 4 Stephen A Pohl [:spohl] 2012-05-24 17:34:00 PDT
(In reply to Josh Aas (Mozilla Corporation) from comment #3)
> Created attachment 627020 [details] [diff] [review]
> fix v1.1
> 
> This seems to work, just switched the IPC call to async in this revision of
> the patch. Can you verify that this works for you, Stephen?

Your patch looks good to me. It correctly initiates the refresh of the plugins and ends up executing the same code path that about:plugins would.

I can't set the flag to "review+" on the attachment though.
Comment 5 Josh Aas 2012-05-24 18:14:15 PDT
(In reply to Stephen A Pohl from comment #4)

> I can't set the flag to "review+" on the attachment though.

Don't worry about it, your comment is enough. Thanks.
Comment 6 Josh Aas 2012-05-29 10:21:20 PDT
try server run:

https://tbpl.mozilla.org/?tree=Try&rev=a80dbd4de767
Comment 7 Josh Aas 2012-05-30 07:24:50 PDT
pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/d047fae0339c
Comment 8 Ed Morley [:emorley] 2012-05-31 06:15:43 PDT
https://hg.mozilla.org/mozilla-central/rev/d047fae0339c
Comment 9 Johnathan Nightingale [:johnath] 2012-06-22 07:00:08 PDT
Josh/Benjamin - how would we feel about uplifting this and bug 686335 across trains? I'm looking for ways to make it easier for Adobe to push out fixes. Are these 14-safe? Are they 13.0.2 safe?
Comment 10 Benjamin Smedberg [:bsmedberg] 2012-06-22 07:22:00 PDT
Comment on attachment 627020 [details] [diff] [review]
fix v1.1

Bug caused by (feature/regressing bug #): unimplemented NPAPI feature for OOPP
User impact if declined: Flash would not be able to trigger the browser to load a new version on upgrade
Testing completed (on m-c, etc.): unknown
Risk to taking this patch (and alternatives if risky): This seems fairly low-risk to me: the same basic codepath happens any time a user loads about:plugins or a page tries to load a new/unknown plugin.
String or UUID changes made by this patch: none

I'd probably also be comfortable taking this in a 13.0.2
Comment 11 Alex Keybl [:akeybl] 2012-06-22 14:01:55 PDT
Comment on attachment 627020 [details] [diff] [review]
fix v1.1

[Triage Comment]
This may need to go into 13.0.2, so it should definitely land in our next beta.
Comment 12 Alex Keybl [:akeybl] 2012-06-22 14:30:26 PDT
Comment on attachment 627020 [details] [diff] [review]
fix v1.1

[Triage Comment]
Please land on mozilla-release asap so that we can test using a tinderbox build. This is not yet shipping in a 13 release, however.
Comment 13 Jason Duell [:jduell] (needinfo me) 2012-06-22 16:46:16 PDT
Landed on beta/release at akeybl's behest:

https://hg.mozilla.org/releases/mozilla-release/rev/486792169136
https://hg.mozilla.org/releases/mozilla-beta/rev/acd0e5e74217
Comment 14 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-06-22 18:02:13 PDT
As with bug 686335...

FYI, QA will be going through the following testplan the next few hours with the latest Nightly:
https://wiki.mozilla.org/Releases/Firefox_13/Test_Plan#Bug_Verifications_2

Please email me at ahughes@mozilla.com with any red flags or revisions you see necessary to giving you the results your need to make any final calls for 13.0.2.

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