758363 - Implement NPN_ReloadPlugins for out-of-process Plugins

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Stephen A Pohl [:spohl]]

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]

Attachment: 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]

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]

Attachment: 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]]

(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]

(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]

try server run:

https://tbpl.mozilla.org/?tree=Try&rev=a80dbd4de767

Comment 7

[Josh Aas]

pushed to mozilla-inbound

http://hg.mozilla.org/integration/mozilla-inbound/rev/d047fae0339c

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d047fae0339c

Comment 9

[Johnathan Nightingale [:johnath]]

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]

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]]

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]]

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]

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

[u279076]

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 [e-mail] 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.