Closed Bug 1198302 Opened 7 years ago Closed 7 years ago
Browser hangs when plugin returns error from NPP
_NEW when Asynchronous Plugin Initialization is enabled
706.69 KB, text/plain
22.26 KB, application/zip
1.27 KB, patch
|Details | Diff | Splinter Review|
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?
Adding crash dump
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
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)
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
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+
You need to log in before you can comment on or make changes to this bug.