Investigate why PPluginWidget's RecvCreate is failure prone

RESOLVED FIXED in Firefox 38

Status

()

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

People

(Reporter: jimm, Assigned: jimm)

Tracking

(Blocks: 1 bug)

Trunk
mozilla38
x86_64
All
Points:
---

Firefox Tracking Flags

(e10sm5+, firefox38 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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)

Updated

3 years ago
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.
(Assignee)

Comment 2

3 years ago
Created attachment 8559442 [details] [diff] [review]
patch
(Assignee)

Comment 4

3 years ago
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)
(Assignee)

Comment 5

3 years ago
(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. :)
(Assignee)

Updated

3 years ago
tracking-e10s: --- → m5+
(Assignee)

Comment 6

3 years ago
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
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Comment 9

3 years ago
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.