Closed Bug 1116457 Opened 5 years ago Closed 4 years ago

Intermittent browser_history_sidebar_search.js | application timed out after 330 seconds with no output on browser/components/places/tests/browser

Categories

(Firefox :: Bookmarks & History, defect)

x86
macOS
defect
Not set
Points:
5

Tracking

()

RESOLVED FIXED
Firefox 47
Tracking Status
e10s + ---
firefox36 --- wontfix
firefox37 --- wontfix
firefox38 --- disabled
firefox-esr31 --- unaffected

People

(Reporter: cbook, Assigned: mak)

References

(Blocks 1 open bug, )

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Rev5 MacOSX Mountain Lion 10.8 fx-team opt test mochitest-e10s-browser-chrome-1

https://treeherder.mozilla.org/logviewer.html#?job_id=1566782&repo=fx-team

19:16:02 INFO - 723 ERROR TEST-UNEXPECTED-TIMEOUT | browser/components/places/tests/browser/browser_history_sidebar_search.js | application timed out after 330 seconds with no output on browser/components/places/tests/browser
Not really sure what's going on here, it looks like the test completes successfully but focus state is confused? The screenshot here: http://mozilla-releng-blobs.s3.amazonaws.com/blobs/fx-team/sha512/cd4ae7602ac3ac6dfb6e8b499fce5904537f6cb4d0d1e412bab909e291f97c1fba6347ff0c46c6755d9708e710ba7bfc7778787ff2a689a57303a9e3093363b7 certainly shows that the window is focused...
Component: General → Bookmarks & History
it's suspiciously connected to the changes in bug 1083269 and the log shows
must wait for focus.
The test doesn't use waitForFocus, so I guess the call is this one
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/browser-test.js#260

maybe there's a problem between the new waitForFocus code and the sidebar contents.
Flags: needinfo?(enndeakin)
Attached patch PatchSplinter Review
Not sure if this will fix this particular issue, but I found an issue where the focus event listener was being added to the parent window instead of the child. This could cause the test to timeout if the sidebar is focused as the focus event will fire at a different window than is expected.

This is a regression from bug 1083269 and this patch makes waitForFocus use the same semantics as before.

Let's try this and see if we see an improvement.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Flags: needinfo?(enndeakin)
Attachment #8545242 - Flags: review?(jmaher)
Attachment #8545242 - Attachment is patch: true
Comment on attachment 8545242 [details] [diff] [review]
Patch

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

I can see how this could help, just one related question.

::: testing/mochitest/tests/SimpleTest/SimpleTest.js
@@ +791,3 @@
>                if (isChildProcess) {
>                    childDesiredWindow.focus();
>                }

in this case will we always want to do the childDesiredWindow.focus()?
Attachment #8545242 - Flags: review?(jmaher) → review+
> I can see how this could help, just one related question.
> 
> ::: testing/mochitest/tests/SimpleTest/SimpleTest.js
> @@ +791,3 @@
> >                if (isChildProcess) {
> >                    childDesiredWindow.focus();
> >                }
> 
> in this case will we always want to do the childDesiredWindow.focus()?

Unsure what you're asking, but this focus() call is only performed when childDesiredWindow is not focused. If it was focused, the conditional 'if (!focused)' a few lines earlier wouldn't match.
in the patch we are adding a listener to childDesiredWindow, then right after it we conditionally focus childDesiredWindow.  I wonder if there is more to do here related to the child window or not.
  childDesiredWindow.addEventListener("focus", focusedOrLoaded, true);
  if (isChildProcess) {
      childDesiredWindow.focus();
  }
  else {
      SpecialPowers.focus(childDesiredWindow);
  }

Here the listener is added to childDesiredWindow, then we focus it, in the first case for the child process and in the second for the parent process. The actual focusing of the window may be asynchronous.
ok, all is well- sorry for the confusion.
https://hg.mozilla.org/integration/mozilla-inbound/rev/83f3a7efed21

Leaving open for now for further investigation.
Keywords: leave-open
Seems to still happen.
Can you give this a points value Neil?
Flags: qe-verify-
Flags: needinfo?(enndeakin)
Flags: firefox-backlog+
Iteration: --- → 38.2 - 9 Feb
Disabled on Linux & OSX e10s given the frequency and lack of progress made for the last month+.

https://hg.mozilla.org/integration/mozilla-inbound/rev/70988272766a
tracking-e10s: --- → ?
Whiteboard: [test disabled on Linux and OSX e10s]
Points: --- → 5
Flags: needinfo?(enndeakin)
Iteration: 38.2 - 9 Feb → 38.3 - 23 Feb
Iteration: 38.3 - 23 Feb → 39.1 - 9 Mar
Iteration: 39.1 - 9 Mar → 39.2 - 23 Mar
Iteration: 39.2 - 23 Mar → ---
Assignee: enndeakin → nobody
Status: ASSIGNED → NEW
Assignee: nobody → mak77
Attachment #8724815 - Flags: review?(adw) → review+
Comment on attachment 8724815 [details]
MozReview Request: Bug 1116457 - Intermittent browser_history_sidebar_search.js. r=adw

https://reviewboard.mozilla.org/r/37165/#review33725

::: browser/components/places/tests/browser/browser_history_sidebar_search.js:40
(Diff revision 1)
> +    // Resete the search

Nit: Reset (no trailing E)
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: leave-open
Resolution: --- → FIXED
Whiteboard: [test disabled on Linux and OSX e10s]
Target Milestone: --- → Firefox 47
You need to log in before you can comment on or make changes to this bug.