Closed Bug 1271819 Opened 4 years ago Closed 3 years ago

Intermittent browser_focus_steal_from_chrome.js | the INPUT element isn't focused by the focus (Test1: content can steal focus) -

Categories

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

49 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox48 --- unaffected
firefox49 --- fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: KWierso, Assigned: Nika)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

This has been failing frequently since bug 1270118 landed. Can you please look, Michael?
Blocks: 1270118
Flags: needinfo?(michael)
I had trouble reasoning about what exactly was happening and when in the original test, so I quickly rewrote it using add_task - I'm running a try to see if the problem reproduces in my rewrite (which could mean that there is a real platform problem :S)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a181ba05a94
I had a hunch that the bug was due to loading not being handled correctly in e10s, but was having trouble reasoning about the way the test worked. I rewrote it, and it seems to work now from my try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a181ba05a941f2a2c67a07f7d5684ac8c207f44 (I repeated OS X 10.10 debug and Linux x64 PGO runs for this bug (which seemed to be the ones causing the most failures) a bunch of times, and didn't get any failures).

Hopefully this actually fixes the failure
Attachment #8776972 - Flags: review?(enndeakin)
Assignee: nobody → michael
Flags: needinfo?(michael)
Comment on attachment 8776972 [details] [diff] [review]
Rewrite browser_focus_steal_from_chrome to avoid race condition

>+        // Confirm that the contents should be able to steal focus from content.
>+        yield ContentTask.spawn(fg, test, test => {
>+          let fm = Components.classes["@mozilla.org/focus-manager;1"]
>+                .getService(Components.interfaces.nsIFocusManager);

I think you can use Services.focus in a ContentTask.


>+
>+          return new Promise(res => {
>+            function f() {
>+              let e = content.document.activeElement;
>+              if (e.tagName != test.tagName) {
>+                dump("... Not Ready Yet ...\n");

Remove the four 'dump' lines. If you want a debug line on failure, count for some time and use info.


>+                setTimeout(f, 10);
>+              } else {
>+                is(fm.focusedElement, e,
>+                   "the foreground tab's " + test.tagName +
>+                   " element isn't focused by the " + test.methodName +
>+                   " (Test1: content can steal focus)");
>+                res();
>+              }
>+            }
>+            f();
>+          });
>+        });

It would also be a good idea after each of these tests to ensure that Services.focus.focusedElement in the parent process is correct as well.
Attachment #8776972 - Flags: review?(enndeakin) → review+
Attachment #8776972 - Attachment is obsolete: true
Pushed by michael@thelayzells.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e330ab0c536
Rewrite browser_focus_steal_from_chrome to avoid race condition, r=enndeakin
https://hg.mozilla.org/mozilla-central/rev/6e330ab0c536
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.