Browser hangs when plugin returns error from NPP_NEW when Asynchronous Plugin Initialization is enabled

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: milan.burda, Assigned: aklotz)

Tracking

40 Branch
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed, firefox42 fixed, firefox43 fixed)

Details

Attachments

(3 attachments)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150812163655

Steps to reproduce:

Return NPERR_INVALID_URL from NPP_New when dom.ipc.plugins.asyncInit is enabled.

Context:

Check if the hosting website is allowed to instantiate our plugin. (For example by getting NPNVdocumentOrigin and comparing to a list of allowed origins)
If the plugin is not allowed to run, we return NPERR_INVALID_URL from NPP_New.


Actual results:

The browser hangs.


Expected results:

The plugin initialization should have failed without hanging the whole browser.
It is happening on both Windows 10 and OS X 10.9.5
Component: Untriaged → Plug-ins
Product: Firefox → Core
OS: Unspecified → Windows 10
Hardware: Unspecified → x86
OS: Windows 10 → All
Hardware: x86 → All
Do you have call stacks for this type of hang?
Flags: needinfo?(milan.burda)
Posted file npp_new_hang.txt
Adding crash dump
Flags: needinfo?(milan.burda)
You can reproduce this issue with the attached NPAPI_HelloWorld_Mac.zip

Steps:
1. Uncomment "return NPERR_INVALID_URL" at the top of NPP_New (PluginAPI.mm)
2. Build (installNPAPIPlugin.sh script creates a symlink to ~/Library/Internet Plug-Ins)
3. Open website/index.html
4. Observer the browser hanging
Posted patch PatchSplinter Review
A pretty straightforward patch. We weren't telling WaitForInit that an init failure had occurred. I'm working on a regression test for bug 1115436 that will catch this in the future.
Assignee: nobody → aklotz
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8653760 - Flags: review?(jmathies)
Attachment #8653760 - Flags: review?(jmathies) → review+
Keywords: checkin-needed
Comment on attachment 8653760 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: async plugin init
[User impact if declined]: browser could hang if JS in page is waiting for plugin to initialize but plugin fails during init
[Describe test coverage new/current, TreeHerder]: Plugin test suite, manually. Regression test for this specific issue is being added in bug 1115436.
[Risks and why]: Low, trivial patch that flips a boolean.
[String/UUID change made/needed]: None
Attachment #8653760 - Flags: approval-mozilla-beta?
Attachment #8653760 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/1a6139dad4b1
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8653760 [details] [diff] [review]
Patch

Taking it into aurora to increase the coverage, Ritu will make the call for beta.
Attachment #8653760 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8653760 [details] [diff] [review]
Patch

Browser hangs are bad, let's uplift the fix to Beta41. The fix seems quite safe.
Attachment #8653760 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Depends on: 1201239
No longer depends on: 1201239
You need to log in before you can comment on or make changes to this bug.