Candy Crush Saga doesn't load with dom.ipc.plugins.asyncInit set to true

RESOLVED FIXED in Firefox 39

Status

()

Core
Plug-ins
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Guilherme Lima, Assigned: aklotz)

Tracking

Trunk
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 disabled, firefox39+ verified, firefox40+ verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
I'm on Nightly with e10s off.
Trying to play Candy Crush Saga (from Facebook), the app won't load, it gets stuck at 78% (when it probably do something different, like load something from elsewhere, I don't know).
I discovered that some plugin streams are being terminated prematurely when async init is on. The crux of the problem is that nsNPAPIPluginStreamListener::OnStartBinding is not always called when you would think it would be called by Necko; sometimes the underlying request has already completed. I've got a patch written that works for Candy Crush Saga but obviously I want to test it out a bit with other content.
Assignee: nobody → aklotz
Blocks: 1116806
Status: NEW → ASSIGNED
status-firefox38: --- → disabled
status-firefox39: --- → affected
OS: Windows 7 → All
Hardware: x86_64 → All
Created attachment 8591871 [details] [diff] [review]
Prevent premature stream termination
Attachment #8591871 - Flags: review?(jmathies)

Comment 3

2 years ago
Comment on attachment 8591871 [details] [diff] [review]
Prevent premature stream termination

Review of attachment 8591871 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/ipc/PluginAsyncSurrogate.cpp
@@ +171,5 @@
>    aFuncs->setvalue = &NPP_SetValue;
>    aFuncs->newstream = &NPP_NewStream;
>    aFuncs->setwindow = &NPP_SetWindow;
>    aFuncs->writeready = &NPP_WriteReady;
> +  aFuncs->write = &NPP_Write;

This is set about 5 lines below to |&PluginModuleParent::NPP_Write| so this line doesn't appear to have any affect. Is that right?
Created attachment 8592314 [details] [diff] [review]
Prevent premature stream termination (r2)

Duh, that's silly of me. Fixed that. I also replaced the two bools that I added to the listener with an enum.
Attachment #8591871 - Attachment is obsolete: true
Attachment #8591871 - Flags: review?(jmathies)
Attachment #8592314 - Flags: review?(jmathies)

Updated

2 years ago
Attachment #8592314 - Flags: review?(jmathies) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdd746ef10bd
https://hg.mozilla.org/mozilla-central/rev/fdd746ef10bd
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox40: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8592314 [details] [diff] [review]
Prevent premature stream termination (r2)

Approval Request Comment
[Feature/regressing bug #]: async plugin init
[User impact if declined]: Plugins don't start properly, particularly Candy Crush Saga or other big games. This patch should also improve stability, so there would be unfixed stability problems.
[Describe test coverage new/current, TreeHerder]: Tested with local and nightly builds with asyncInit enabled.
[Risks and why]: Low. Simple patch that makes the browser more conservative with respect to terminating plugin streams.
[String/UUID change made/needed]: None
Attachment #8592314 - Flags: approval-mozilla-aurora?
Florin, can your team test this fix on nightly before we uplift it?  

Aaron, what other games might be useful to test?
Flags: needinfo?(florin.mezei)
Flags: needinfo?(aklotz)
Tracking for 39+
tracking-firefox39: --- → +
tracking-firefox40: --- → +
(In reply to Liz Henry (:lizzard) from comment #8)
> Aaron, what other games might be useful to test?

Any game that is takes a while to start because it has a bunch of assets that need to be downloaded. FarmVille and other popular Facebook games are also good candidates.
Flags: needinfo?(aklotz)
Verified as fixed using the following environment:

FF 40
Build Id: 20150416030209
OS: Win 7 x64
status-firefox40: fixed → verified
Flags: needinfo?(florin.mezei)
Comment on attachment 8592314 [details] [diff] [review]
Prevent premature stream termination (r2)

Approving for uplift to aurora.
Attachment #8592314 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/7c2d250ea3f2
status-firefox39: affected → fixed
Backed out for test_pluginstream_err.html crashes.
https://hg.mozilla.org/releases/mozilla-aurora/rev/a1d5331b1823

https://treeherder.mozilla.org/logviewer.html#?job_id=772938&repo=mozilla-aurora
status-firefox39: fixed → affected
Ha! Looks like we hit bug 1155503.
Depends on: 1155503
u-a-f according to ASAN:
https://treeherder.mozilla.org/logviewer.html#?job_id=773028&repo=mozilla-aurora
Relanded along with bug 1155503 uplift.
https://hg.mozilla.org/releases/mozilla-aurora/rev/6b316eabf6e1
status-firefox39: affected → fixed
I verified this fix using:

FF 39.0b2
Build Id: 20150601171003
OS: Win 7 x64, Mac Os X 10.9.5

and I've encountered an issue, candy crush saga is not completely loading when using a new profile, I've logged bug 1170486 for this issues.
status-firefox39: fixed → verified
You need to log in before you can comment on or make changes to this bug.