Closed Bug 1129575 Opened 6 years ago Closed 6 years ago

Investigate why PPluginWidget's RecvCreate is failure prone

Categories

(Core :: Plug-ins, defect)

x86_64
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
e10s m5+ ---
firefox38 --- fixed

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(1 file)

This call fails a lot, but it shows up as a failure in SendGetNativePluginPort, which is the first sync call we make after Create is sent over async.
Assignee: nobody → jmathies
I remember seeing this when I was testing the phoronix page. The STR was roughly as follows:

1. Create a bookmarks folder that contains a bunch of phoronix articles (maybe 20 or 30).
2. Select "Open All in Tabs" for that folder.
3. (Not sure about this) Quickly close the tabs while they're still loading.

Most of the time I would get a hang (which should not be fixed), but sometimes I think I got this failure.
Attached patch patchSplinter Review
Comment on attachment 8559442 [details] [diff] [review]
patch

This call is failing at a rather high rate. I'm guessing the cause is a bad parent, which we retrieve from TabParent right before we try to create the window. So I'm relaxing this failure case. The other two potential failures (oom, and a failure in native create window calls) still abort. I've also switched this back to sync so we have the return result in the content process.
Attachment #8559442 - Flags: review?(roc)
(In reply to Bill McCloskey (:billm) from comment #1)
> I remember seeing this when I was testing the phoronix page. The STR was
> roughly as follows:
> 
> 1. Create a bookmarks folder that contains a bunch of phoronix articles
> (maybe 20 or 30).
> 2. Select "Open All in Tabs" for that folder.
> 3. (Not sure about this) Quickly close the tabs while they're still loading.
> 
> Most of the time I would get a hang (which should not be fixed), but
> sometimes I think I got this failure.

I've been testing this on hulu front page clip links, which sounds similar. I've reproduced the abort once in nightly there, but haven't seen it with these changes. Crash stats will be the ultimate judge though. :)
tracking-e10s: --- → m5+
Comment on attachment 8559442 [details] [diff] [review]
patch

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

::: dom/ipc/TabChild.cpp
@@ +3487,5 @@
>      NS_WARNING("Creating native plugin widget on the chrome side failed.");
>    }
> +  *aOut = pluginWidget.get();
> +  pluginWidget.forget();
> +  return rv;

This code can be shortened to:

pluginWidget.forget(aOut);
return rv;

and it avoids an assert in debug builds.
https://hg.mozilla.org/mozilla-central/rev/cadacf4bc878
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
This worked well, 99% of these aborts are now gone.
You need to log in before you can comment on or make changes to this bug.