1674743 - Calling window.focus() in OOP iframe fires focus event asynchronously

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[Edgar Chen [:edgar]]

Attachment: test.html

Fission

With fission enabled, the focus event is dispatched asynchronously after calling window.focus() in an OOP iframe.

onload
before calling window.focus()
after calling window.focus()
window.onfocus: target: [object HTMLDocument], eventPhase: 1
window.onfocus: target: [object Window], eventPhase: 2
window.onfocus: target: [object Window], eventPhase: 2

non-Fission

The focus event used to be dispatched synchronously while calling window.focus() in the iframe.

onload
before calling window.focus()
window.onfocus: target: [object HTMLDocument], eventPhase: 1
window.onfocus: target: [object Window], eventPhase: 2
window.onfocus: target: [object Window], eventPhase: 2
after calling window.focus()

Comment 1

[Edgar Chen [:edgar]]

window.focus() on a non-OOP iframe would call nsContentUtils::RequestFrameFocus synchronously, https://searchfox.org/mozilla-central/rev/7b07725fe9dc5d59a525990fc1dba78bc8b82af1/dom/base/nsGlobalWindowOuter.cpp#5143, and then dispatch focus event synchronously.

But window.focus() on an OOP iframe sends an IPC message to its parent frame instead, https://searchfox.org/mozilla-central/rev/7b07725fe9dc5d59a525990fc1dba78bc8b82af1/dom/base/nsGlobalWindowOuter.cpp#5131-5136, then wait for parent to send IPC back, the focus event is fired asynchronously due to the bounce of the IPC.

Comment 2

[Edgar Chen [:edgar]]

Henri, do you have any idea how should we deal with this?

Comment 3

[Henri Sivonen (:hsivonen)]

Looks like it's synchronous in Chrome. Ouch.

(In reply to Edgar Chen [:edgar] from comment #2)

Henri, do you have any idea how should we deal with this?

Unclear. One option would be to try to fire the event synchronously in the process containing the frame and then somehow suppress the event from firing when the framed content catches up. It would be nice to know what Chrome actually does.

Comment 4

[Anne (:annevk)]

That is interesting, those Chrome results seem different from https://github.com/whatwg/html/issues/3506. Does that mean they had to change things here or are we missing something?

Comment 5

[Edgar Chen [:edgar]]

(In reply to Anne (:annevk) from comment #4)

That is interesting, those Chrome results seem different from https://github.com/whatwg/html/issues/3506.

This seems a different case.

• This bug is filed for window.focus() called in iframe process, and Chrome handles and fires focus event synchronously in the iframe process, but we do it asynchronously.
• The spec one is for iframe.contentWindow.focus() called in parent frame process, both Chrome and us handle focus in iframe process asynchronously.

Comment 6

[Henri Sivonen (:hsivonen)]

I discussed this with Neha and the conclusion was to try a fix similar to https://hg.mozilla.org/mozilla-central/rev/efecc359f305 . That is, let's try firing the event synchronously and suppressing the async event that we currently fire.

Comment 7

[Edgar Chen [:edgar]]

I have a local patch that calls nsFocusManager::WindowRaised before https://searchfox.org/mozilla-central/rev/e75e8e5b980ef18f4596a783fbc8a36621de7d1e/dom/base/nsGlobalWindowOuter.cpp#5132, then the focus event would be dispatched synchronously, but we still get async event after we send the IPC to parent frame, I feel that it is somehow related to the bug 1674702 that parent frame tries to blur and focus the OOP frame when it receives the focus IPC.

Comment 8

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #6)

I discussed this with Neha and the conclusion was to try a fix similar to https://hg.mozilla.org/mozilla-central/rev/efecc359f305 . That is, let's try firing the event synchronously and suppressing the async event that we currently fire.

Oops. I was commenting on a different issue than the one on file. I was commenting on how the focus event fires in the framer when the framer focuses an OOP iframe. We have differences from Chrome, but the difference may be the we don't fire the event at all.

Comment 9

[Edgar Chen [:edgar]]

(In reply to Henri Sivonen (:hsivonen) from comment #8)

Oops. I was commenting on a different issue than the one on file. I was commenting on how the focus event fires in the framer when the framer focuses an OOP iframe. We have differences from Chrome, but the difference may be the we don't fire the event at all.

Are you talking about something like https://codepen.io/edgarchen-the-decoder/pen/GRqGNmo?
I also noticed that chrome fires blur event synchronously in framer when the framer focuses an OOP iframe, and we don't fire blur event at all.

Comment 10

[Henri Sivonen (:hsivonen)]

Along those lines, but I was thinking of iframe.onfocus when calling iframe.contentWindow.focus(). It looks like we don't fire that event at all in the in-process case. I filed bug 1675484.

Comment 11

[Edgar Chen [:edgar]]

Attachment: Bug 1674743 - Synchronously raise window when window.focus() called in OOP iframe;

Comment 12

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=e68d969f4caa55b6e8db5c9d36576bca31b32040

Comment 13

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4bcd5661c4c751a04373b1856c151d2d9cb2f23e

Comment 14

[Edgar Chen [:edgar]]

The patch breaks dom/tests/mochitest/bugs/test_bug369306.html, https://treeherder.mozilla.org/logviewer?job_id=321230422&repo=try&lineNumber=3971.

Comment 15

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=81019eca34e3067bdbdcdbe79499bc633a6d88dc

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17658a2394cf
Synchronously raise window when window.focus() called in OOP iframe; r=hsivonen

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/26486 for changes under testing/web-platform/tests

Comment 18

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/17658a2394cf

Comment 19

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 20

[Dennis Schubert [:denschub]]

Edgar, it looks like the WPT in focus-restoration-in-different-site-iframes.html are unstable in Chrome: https://github.com/web-platform-tests/wpt/pull/26486/checks?check_run_id=1420212453

Could you have a look at that and check if this is a Chrome-bug or an issue with the test that would require changes in this bug?

Comment 21

[Edgar Chen [:edgar]]

We have a similar intermittent failure in our CI, see bug 1642889.
Let's see if we found something in bug 1642889 first.

Comment 22

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by jgraham