Closed Bug 1272823 Opened 8 years ago Closed 10 months ago

Intermittent browser_html_tooltip-03.js | Test timed out

Categories

(DevTools :: Shared Components, defect, P3)

Product:
DevTools
Component:
Shared Components
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: philor, Unassigned)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Bug 1272823 - fix intermittent failures for html tooltip & autocomplete tests;
58 bytes, text/x-review-board-request
Details 
I knew I was going to do that eventually.
Product: Developer Documentation → Firefox
Component: Developer Tools → Developer Tools: Shared Components
Priority: -- → P3
We are trying to click and focus a XUL textbox in the beginning of the test. Most likely needs to wait for MozAfterPaint.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=89d6be353540
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Attachment #8758043 - Flags: review?(poirot.alex)
Comment on attachment 8758043 [details]
Bug 1272823 - fix intermittent failures for html tooltip & autocomplete tests;

https://reviewboard.mozilla.org/r/56408/#review52984

::: devtools/client/shared/test/browser_html_tooltip-03.js:35
(Diff revision 1)
>  add_task(function* () {
>    yield addTab("about:blank");
> -  let [,, doc] = yield createHost("bottom", TEST_URI);
> +  let [, win, doc] = yield createHost("bottom", TEST_URI);
> +
> +  // Wait for UI paint before trying to click & focus the textbox.
> +  yield once(win, "MozAfterPaint");

I don't see why this particular test needs a dedicated wait. It looks like something that would help prevent races in others tests as well.
What do you think about moving that to createHost?

Also, this is weird. We try to click and focus in the beginning of the test, but createHost already waits for DOMContentLoaded. It looks like it should be enough. I don't see why we would need to wait more. But I'm not surprised as XUL is involved here...
I'm wondering if that just XUL, or if we should wait for another event? load event?
Comment on attachment 8758043 [details]
Bug 1272823 - fix intermittent failures for html tooltip & autocomplete tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/56408/diff/1-2/
Attachment #8758043 - Attachment description: MozReview Request: Bug 1272823 - wait for MozAfterPaint in browser_html_tooltip-03;r=ochameau → Bug 1272823 - fix intermittent failures for html tooltip & autocomplete tests;
Attachment #8758043 - Flags: review?(poirot.alex)
(In reply to Alexandre Poirot [:ochameau] from comment #4)
> Comment on attachment 8758043 [details]
> Bug 1272823 - fix intermittent failures for html tooltip & autocomplete
> tests;
> 
> https://reviewboard.mozilla.org/r/56408/#review52984
> 
> ::: devtools/client/shared/test/browser_html_tooltip-03.js:35
> (Diff revision 1)
> >  add_task(function* () {
> >    yield addTab("about:blank");
> > -  let [,, doc] = yield createHost("bottom", TEST_URI);
> > +  let [, win, doc] = yield createHost("bottom", TEST_URI);
> > +
> > +  // Wait for UI paint before trying to click & focus the textbox.
> > +  yield once(win, "MozAfterPaint");
> 
> I don't see why this particular test needs a dedicated wait. It looks like
> something that would help prevent races in others tests as well.
> What do you think about moving that to createHost?
> 
> Also, this is weird. We try to click and focus in the beginning of the test,
> but createHost already waits for DOMContentLoaded. It looks like it should
> be enough. I don't see why we would need to wait more. But I'm not surprised
> as XUL is involved here...
> I'm wondering if that just XUL, or if we should wait for another event? load
> event?

As you suggested, waiting for MozAfterPaint was only working out for me because it was delaying a bit the test execution. After looking at your changes for Bug 1267414, I realized that all html-tooltip and inplace-editor-autocomplete tests were calling addTab("about:blank") on test startup for no reason, before calling createHost.

Locally, removing the addTab("about:blank") seems to fix the intermittent failures.

However I am still not sure about the reason why it solves the issue. The addTab method waits for the load event, so the new tab should be completely loaded before calling createHost. Any idea ?

(try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=131e069b86134de56ea1f1afb38dd66d63dfb90f)
If this actually works, it would solve the following intermittents as well :
- Bug 1273656 - Intermittent browser_html_tooltip-02.js | Test timed out
- Bug 1277441 - Intermittent browser_inplace-editor_autocomplete_01.js | Test timed out
- Bug 1277657 - Intermittent browser_inplace-editor_autocomplete_02.js | Correct value is autocompleted - Got , expected block | Popup is closed | Test timed out
- Bug 1281895 - Intermittent devtools/client/shared/test/browser_inplace-editor_autocomplete_offset.js | Test timed out -
Comment on attachment 8758043 [details]
Bug 1272823 - fix intermittent failures for html tooltip & autocomplete tests;

https://reviewboard.mozilla.org/r/56408/#review58376

Are you able to reproduce it locally somewhat easily?
Is this still timeouting on the focusNode() call and the wait for focus event?
Could you look into activeElement before the focus event wait?
May be it is already focused? Which seems unlikely. Or may be some other document steal the focus?
It could also be worth checking for the activeElement on the top level document see if it changes...

require("sdk/timers").setTimeout(function () {
  dump(doc.activeElement + " -- " + document.activeElement+"\n");
}, 1000);

The tab document may be stealing the focus... But addTab should be waiting for document load, I would expect focus to be done by the end of this event?
Attachment #8758043 - Flags: review?(poirot.alex)
(In reply to Alexandre Poirot [:ochameau] from comment #10)
> Comment on attachment 8758043 [details]
> Bug 1272823 - fix intermittent failures for html tooltip & autocomplete
> tests;
> 
> https://reviewboard.mozilla.org/r/56408/#review58376
> 
> Are you able to reproduce it locally somewhat easily?

I reproduce the issue almost every time I run "./mach mochitest browser_html_tooltip-03.js --run-until-failure" , on OSX opt build.

> Is this still timeouting on the focusNode() call and the wait for focus
> event?

Yes still timeouting when waiting for focus.

> Could you look into activeElement before the focus event wait?
> May be it is already focused? Which seems unlikely. Or may be some other
> document steal the focus?

Before trying to move the focus to the textbox, the parent window has the focus. 
And that remains true when the test is successful or when it timeouts.

> It could also be worth checking for the activeElement on the top level
> document see if it changes...
> 
> require("sdk/timers").setTimeout(function () {
>   dump(doc.activeElement + " -- " + document.activeElement+"\n");
> }, 1000);
> 

The top level document's activeElement is interesting. 
When is passes, it is the <browser> element before we try to focus, and it remains the same after a delay.
When it fails, it is the topmost <window#mainwindow> before we try to focus, and becomes the <browser> element a bit later.

After some more logging, the focus in the top document moves as follows:
- test start : <browser>
- after yield addTab : <window#main-window>
- after waiting few ms : <browser>

(but this is test dependent: if the previous test left the focus on the tabs for instance, the behaviour will be different)

> The tab document may be stealing the focus... But addTab should be waiting
> for document load, I would expect focus to be done by the end of this event?

I also tried using the "load" event on the iframe created in the createHost() method, no improvement.
I tried using BrowserTestUtils.browserLoaded, it improves the situation, but will still fail from time to time.

Forcing the focus of gBrowser.selectedBrowser before leaving addTab seems to work reliably, but not sure this is something we should do.

I would like to dig more into the focus management after adding/selecting a tab, but I can't spend more time on this right now.
Product: Firefox → DevTools
Assignee: jdescottes → nobody
Status: ASSIGNED → NEW
Severity: normal → S3

https://wiki.mozilla.org/Bug_Triage#Intermittent_Test_Failure_Cleanup
For more information, please visit BugBot documentation.

Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: