Closed Bug 1335057 Opened 7 years ago Closed 7 years ago

Intermittent dom/base/test/browser_bug1058164.js | Test timed out -

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: Mardak)

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:race])

Attachments

(1 file)

This failure comes and goes. A review of OrangeFactor for the last month shows 3 short-lived peaks of very-frequent failures with periods of low-frequency failures in between. 

Failures have subsided again, but if this peaks again, we should take action right away.
Whiteboard: [stockwell unknown]
Priority: -- → P5
Hi Mike,
The intermittent failure rate became rising and unstable in the past month. Do you think you have time to take a look at this soon-ish?
Flags: needinfo?(mconley)
This hasn't been seen for about a week, at least according to orange factor. I'm going to clear this needinfo for now. If it picks up again, let me know.
Flags: needinfo?(mconley)
(In reply to Mike Conley (:mconley) from comment #18)
> This hasn't been seen for about a week, at least according to orange factor.
> I'm going to clear this needinfo for now. If it picks up again, let me know.

It is back! Failures started again on Aug 4 and continue now, quite frequently on osx.
Flags: needinfo?(mconley)
(In reply to Ed Lee :Mardak from comment #23)
> Created attachment 8895470 [details]
> Bug 1335057 - Avoid intermittent bug1058164.js test timed out by racing.
This doesn't actually fix whatever underlying problem, but it seems to at least avoid the test timed out…

The 5000 matches BrowserTestUtils.waitForCondition's default 5 seconds. Not sure if there's a better number to use.
Comment on attachment 8895470 [details]
Bug 1335057 - Fix intermittent bug1058164.js test timed out by conditionally waiting.

https://reviewboard.mozilla.org/r/166666/#review172386

Thanks for looking at this Mardak! See below.

::: dom/base/test/browser_bug1058164.js:92
(Diff revision 1)
> -  await new Promise((resolve) => {
> +  await Promise.race([new Promise(resolve => {
>      emptyBrowser.addEventListener("pageshow", function() {
> +      info("got pageshow from empty");
>        resolve();
>      }, {once: true});
> -  });
> +  }), new Promise(resolve => setTimeout(resolve, 5000))]);
> +  info("empty vs timeout race resolved");

I dug into this a little bit, and it looks like we can actually detect the visibility state of a document via `nsIDOMDocument.hidden`.

So instead of setTimeout'ing here, what I recommend we do is to use a ContentTask to check the `content.document.QueryInterface(Ci.nsIDOMDocument).hidden` state. If hidden, await a `pageshow` event. Otherwise, we can resolve right away.
Attachment #8895470 - Flags: review?(mconley) → review-
Comment on attachment 8895470 [details]
Bug 1335057 - Fix intermittent bug1058164.js test timed out by conditionally waiting.

https://reviewboard.mozilla.org/r/166666/#review172414

Thanks!

::: dom/base/test/browser_bug1058164.js:87
(Diff revision 2)
> -  // use BrowserTestUtils.waitForEvent here because we're using the
> -  // e10s add-on shims in the e10s-case. I'm doing this because I couldn't
> -  // find a way of sending down a frame script to the newly opened windows
> -  // and tabs fast enough to attach the event handlers before they were
> -  // fired.
> -  await new Promise((resolve) => {
> +  await ContentTask.spawn(emptyBrowser, {}, () => new Promise(resolve => {
> +    const {visibilityState} = content.document;
> +    info(`emptyBrowser's visibilityState: ${visibilityState}`);
> +    if (visibilityState === "hidden") {
> +      content.addEventListener("pageshow", resolve);
> +    } else {
> -    emptyBrowser.addEventListener("pageshow", function() {
>        resolve();
> -    }, {once: true});
> -  });
> +    }
> +  }));

Something like this is probably simpler:

```js

await ContentTask.spawn(emptyBrowser, null, async function() {
  if (content.document.visibilityState === "hidden") {
    await ContentTaskUtils.waitForEvent(content, "pageshow");
  }
});

```
Attachment #8895470 - Flags: review?(mconley) → review+
Pushed by edilee@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/27db3f759f7a
Fix intermittent bug1058164.js test timed out by conditionally waiting. r=mconley
Flags: needinfo?(mconley)
Whiteboard: [stockwell needswork:DOM] → [stockwell fixed:race]
https://hg.mozilla.org/mozilla-central/rev/27db3f759f7a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee: nobody → edilee
Whiteboard: [stockwell needswork:DOM] → [stockwell fixed:race]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.