Closed Bug 1102900 Opened 10 years ago Closed 5 years ago

Intermittent browser_989751_subviewbutton_class.js | Test timed out - expected PASS

Categories

(Firefox :: General, defect, P3)

Product:
Firefox
Component:
General
Platform:
x86
Linux
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: cbook, Assigned: Gijs)

References

(
URL
)

Details

(Keywords: intermittent-failure, Whiteboard: [test disabled on Linux e10s][leave open])

Attachments

(1 file)

fix e10s pgo issues with customizableui test,
3.84 KB, patch
mossop
: feedback+
Details | Diff | Splinter Review 
Ubuntu VM 12.04 b2g-inbound pgo test mochitest-e10s-browser-chrome-1


https://treeherder.mozilla.org/ui/logviewer.html#?job_id=880069&repo=b2g-inbound

06:36:24 INFO - 579 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/customizableui/test/browser_989751_subviewbutton_class.js | Test timed out - expected PASS
Between this bug and the See Alsos being marked, this test currently fails ~50% of the time on Linux e10s. Sorry for this slipping between the cracks CC-wise :(

Disabled:
https://hg.mozilla.org/integration/fx-team/rev/b40d31806495
Flags: needinfo?(gijskruitbosch+bugs)
See Also: → 1104745, 1104761
Whiteboard: [test disabled on Linux e10s][leave open]
I don't like this patch, but it works locally. In particular, I think (a) the test framework should take care about the content process being ready, and (b) it's weird that about:blank never gets readyState: 'complete'. Dave, I'm told you know this stuff better than most, can you help?

trypush: remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=decf3493debc
Attachment #8569249 - Flags: feedback?(dtownsend)
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs Kruitbosch from comment #102)
> Created attachment 8569249 [details] [diff] [review]
> fix e10s pgo issues with customizableui test,
> 
> I don't like this patch, but it works locally. In particular, I think (a)
> the test framework should take care about the content process being ready,
> and (b) it's weird that about:blank never gets readyState: 'complete'. Dave,
> I'm told you know this stuff better than most, can you help?

I'm a bit confused by what is going on here and what problem you're trying to solve, can you elaborate?
(In reply to Dave Townsend [:mossop] from comment #103)
> (In reply to :Gijs Kruitbosch from comment #102)
> > Created attachment 8569249 [details] [diff] [review]
> > fix e10s pgo issues with customizableui test,
> > 
> > I don't like this patch, but it works locally. In particular, I think (a)
> > the test framework should take care about the content process being ready,
> > and (b) it's weird that about:blank never gets readyState: 'complete'. Dave,
> > I'm told you know this stuff better than most, can you help?
> 
> I'm a bit confused by what is going on here and what problem you're trying
> to solve, can you elaborate?

Basically, the test opens a popup and then waits for popupshown, and it then checks the content of the subview. The issue in the e10s pgo situation is that something (and AFAICT, in my local case, this is the load of about:blank in the content process) causes the popup to close again (presumably because focus changes). Then the test checks the content of the popup, which is closed and was therefore emptied, and fails.

I'm trying to fix this by making the popup opening wait for the content to have loaded.

Weirdly, in my local testing, about:blank never reaches readyState=complete.
Comment on attachment 8569249 [details] [diff] [review]
fix e10s pgo issues with customizableui test,

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

Yeesh I left this a long time. I never really got this reproducing on my linux box so I don't know what to say but if this fixes it then we should do it.

::: browser/components/customizableui/test/head.js
@@ +575,5 @@
> +      loadPromiseResolve();
> +    }
> +  });
> +  return loadPromise;
> +}

This method is racy, the addEventListener shims rely on sync messaging so we could end up getting a load event before the content task returns a ready state saying that it is already loaded leading to an attempt to remove the already removed event listener. I think you should just combine these. Make a content task that checks the readyState and if necessary wants for load before returning. Bonus points for adding a simple promiseEvent method to somewhere content tasks share like bug 1144797 is looking for.
Attachment #8569249 - Flags: feedback?(dtownsend) → feedback+
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3

This was re-enabled by https://hg.mozilla.org/mozilla-central/rev/824d9caf0aea, but never got marked as fixed :-(

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Assignee: nobody → gijskruitbosch+bugs
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: