Here's the stack: select@chrome://global/content/bindings/textbox.xml:115:11 focusAndSelectUrlBar@chrome://browser/content/browser.js:2100:5 openTab/</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:870:11 promise callback*openTab/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:866:14 promise callback*openTab@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:836:12 init/</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-containers/index.js:259:21 raw@resource://gre/modules/ExtensionCommon.jsm:1376:25 receiveMessage@resource://gre/modules/ExtensionChild.jsm:363:24 _callHandlers/responses<@resource://gre/modules/MessageChannel.jsm:587:16 _callHandlers@resource://gre/modules/MessageChannel.jsm:585:21 _handleMessage/deferred.promise<@resource://gre/modules/MessageChannel.jsm:645:7 _handleMessage@resource://gre/modules/MessageChannel.jsm:642:24 receiveMessage@resource://gre/modules/MessageChannel.jsm:165:5

The relevant line here is: this.inputField.select(); I'm not sure, but I suspect this hides a focus call. Or maybe the stack is slightly incorrect. The stack here looks like it's related to containers, so bug 1349211 seems related.

select() implies focus().

Despite bug 1369140 landing, and making focus not cause a layout flush, apparently .select() still does.

This no longer appears to cause layout flushes.

STR: 1. install ohnoreflow, set threshold to 0, enable 2. open a tab with web content 3. open find bar 4. hit accel-t to open a new tab (which will focus the url bar).

Going to work on this instead of bug 1462374.

The flush here isn't coming directly from focus handling code itself, it's coming from IMEStateManager::OnFocusInEditor. In other words, if there's an IME content observer (not sure what that is) and focus moves into an editor, we try to flush pending notifications, ie we attempt to cause a sync reflow. Masayuki-san, can you help? What's the reason IME needs to cause a reflow when editors (even plaintext ones) get focused? Could we maybe do the same thing as focus code itself (ie only cause what's essentially a style flush), or check if there's a frame for the editor before forcing a style flush (so we don't do so except for nodes that have only just been made visible or something), or could we even change the IME/Editor code to allow an async reflow instead of a sync one?

To be more precise, there's a JS stack in comment #0 which more or less still applies. From HTMLInputElement.select() called from JS, we end up with the following C++ stack for the reflow: #0 0x000000010d4ea324 in nsDocShell::NotifyReflowObservers(bool, double, double) at mozilla-unified/docshell/base/nsDocShell.cpp:1823 #1 0x000000010d4ea5f0 in non-virtual thunk to nsDocShell::NotifyReflowObservers(bool, double, double) () #2 0x000000010c700a8d in mozilla::PresShell::DidDoReflow(bool) at mozilla-unified/layout/base/PresShell.cpp:8850 #3 0x000000010c706ec9 in mozilla::PresShell::ProcessReflowCommands(bool) at mozilla-unified/layout/base/PresShell.cpp:9207 #4 0x000000010c706864 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) at mozilla-unified/layout/base/PresShell.cpp:4348 #5 0x000000010b0d47a3 in nsIPresShell::FlushPendingNotifications(mozilla::ChangesToFlush) [inlined] at mozilla-unified/layout/base/nsIPresShell.h:577 #6 0x000000010b0d4758 in nsIDocument::FlushPendingNotifications(mozilla::ChangesToFlush) at mozilla-unified/dom/base/nsDocument.cpp:7660 #7 0x000000010bc89b7f in mozilla::ContentEventHandler::InitBasic() [inlined] at mozilla-unified/dom/events/ContentEventHandler.cpp:255 #8 0x000000010bc89b67 in mozilla::ContentEventHandler::InitCommon(mozilla::SelectionType) at mozilla-unified/dom/events/ContentEventHandler.cpp:327 #9 0x000000010bc89e11 in mozilla::ContentEventHandler::Init(mozilla::WidgetQueryContentEvent*) at mozilla-unified/dom/events/ContentEventHandler.cpp:402 #10 0x000000010bc8cb8c in mozilla::ContentEventHandler::OnQuerySelectedText(mozilla::WidgetQueryContentEvent*) at mozilla-unified/dom/events/ContentEventHandler.cpp:1412 #11 0x000000010bcb17b3 in mozilla::IMEContentObserver::UpdateSelectionCache() at mozilla-unified/dom/events/IMEContentObserver.cpp:1564 #12 0x000000010bcb2190 in mozilla::IMEContentObserver::IMENotificationSender::SendFocusSet() at mozilla-unified/dom/events/IMEContentObserver.cpp:1969 #13 0x000000010bcb1db9 in mozilla::IMEContentObserver::IMENotificationSender::Run() at mozilla-unified/dom/events/IMEContentObserver.cpp:1858 #14 0x000000010bcb6fdc in mozilla::IMEContentObserver::TryToFlushPendingNotifications(bool) [inlined] at mozilla-unified/dom/events/IMEContentObserver.cpp:1725 #15 0x000000010bcb6f87 in mozilla::IMEStateManager::OnFocusInEditor(nsPresContext*, nsIContent*, mozilla::EditorBase&) at mozilla-unified/dom/events/IMEStateManager.cpp:893 #16 0x000000010c5caa1b in mozilla::EditorEventListener::Focus(mozilla::InternalFocusEvent*) at mozilla-unified/editor/libeditor/EditorEventListener.cpp:1132 #17 0x000000010bca8b52 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) at mozilla-unified/dom/events/EventListenerManager.cpp:1106 #18 0x000000010bca9699 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) at mozilla-unified/dom/events/EventListenerManager.cpp:1308 #19 0x000000010bca2742 in mozilla::EventListenerManager::HandleEvent(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) [inlined] at builds/opt/dist/include/mozilla/EventListenerManager.h:390 #20 0x000000010bca2736 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) at mozilla-unified/dom/events/EventDispatcher.cpp:424 #21 0x000000010bca2130 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) at mozilla-unified/dom/events/EventDispatcher.cpp:641 #22 0x000000010bca3c18 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) at mozilla-unified/dom/events/EventDispatcher.cpp:1156 #23 0x000000010b11efb2 in FocusBlurEvent::Run() at mozilla-unified/dom/base/nsFocusManager.cpp:2077 #24 0x000000010afa234a in nsContentUtils::AddScriptRunner(already_AddRefed<nsIRunnable>) [inlined] at mozilla-unified/dom/base/nsContentUtils.cpp:5682 #25 0x000000010afa2341 in nsContentUtils::AddScriptRunner(nsIRunnable*) at mozilla-unified/dom/base/nsContentUtils.cpp:5689 #26 0x000000010b0f0ffb in nsFocusManager::FireFocusOrBlurEvent(mozilla::EventMessage, nsIPresShell*, nsISupports*, bool, bool, mozilla::dom::EventTarget*) at mozilla-unified/dom/base/nsFocusManager.cpp:2250 #27 0x000000010b0f05f5 in nsFocusManager::SendFocusOrBlurEvent(mozilla::EventMessage, nsIPresShell*, nsIDocument*, nsISupports*, unsigned int, bool, bool, mozilla::dom::EventTarget*) at mozilla-unified/dom/base/nsFocusManager.cpp:2215 #28 0x000000010b0ee6c8 in nsFocusManager::Focus(nsPIDOMWindowOuter*, mozilla::dom::Element*, unsigned int, bool, bool, bool, bool, nsIContent*) at mozilla-unified/dom/base/nsFocusManager.cpp:1996 #29 0x000000010b0eaf1b in nsFocusManager::SetFocusInner(mozilla::dom::Element*, int, bool, bool) at mozilla-unified/dom/base/nsFocusManager.cpp:1399 #30 0x000000010b0eb463 in nsFocusManager::SetFocus(mozilla::dom::Element*, unsigned int) at mozilla-unified/dom/base/nsFocusManager.cpp:514 #31 0x000000010bd67acc in mozilla::dom::HTMLInputElement::Select() at mozilla-unified/dom/html/HTMLInputElement.cpp:3302 #32 0x000000010b98a4fa in mozilla::dom::HTMLInputElement_Binding::select(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLInputElement*, JSJitMethodCallArgs const&) at builds/opt/dom/bindings/HTMLInputElementBinding.cpp:3447

(In reply to :Gijs (he/him) from comment #7) > The flush here isn't coming directly from focus handling code itself, it's > coming from IMEStateManager::OnFocusInEditor. In other words, if there's an > IME content observer (not sure what that is) IMEContentObserver observes any changes of text, scroll position, selection ranges, etc in/of focused editor and lets native IME know the changes via nsIWidget. > and focus moves into an editor, > we try to flush pending notifications, ie we attempt to cause a sync reflow. > > Masayuki-san, can you help? What's the reason IME needs to cause a reflow > when editors (even plaintext ones) get focused? First, we need to initialize IME context for focused editor ASAP when editor gets focus. Otherwise, user may type first character before that. If it occurs, the typed character is directly typed to the focused editor even if IME is open. So, in such case, second character and later are only composed with IME. This is really annoying symptom. Next, IME and/or IME framework of OS wants to know selection range information when its context is initialized. For example, IME may want to move its owning window to insertion point, may want to prepare its dictionary with selected text, etc. Therefore, we need to send selection information to main process when notifying IME of focus. > Could we maybe do the same > thing as focus code itself (ie only cause what's essentially a style flush), > or check if there's a frame for the editor before forcing a style flush (so > we don't do so except for nodes that have only just been made visible or > something), or could we even change the IME/Editor code to allow an async > reflow instead of a sync one? In most cases, when an editor gets focus, editor must not have pending layout information which causes move selected text or something. So, I *guess* that it's possible to make ContentEventHandler work without flushing layout only when notifying focus. Even if unfortunately there is a big pending reflow, after it occurs, IMEContentObserver lets IME know the new position.

Sorting out some things: 1. I believe that we cannot make focus notification asynchronous since user may type some characters before we initialize IME context. 2. I guess that we could put off only flushing layout at focus change. I.e., use previous coordinates for each character rectangle until we'll notify IME of layout change. 3. On Windows, some IMEs (TIPs) are not aware of asynchronous layout even after Windows 10 fixes the bug of TSF. 4. On macOS, we cannot notify IME of layout change even with [inputContext invalidateCharacterCoordinates]. Therefore, the first result for retrieving character rectangle(s) is really important. So, perhaps, we cannot put off flushing layout even at focus.

> 3. On Windows, some IMEs (TIPs) are not aware of asynchronous layout even after Windows 10 fixes the bug of TSF. I meant that some IMEs are not async-layout aware, but we can notify IMEs of layout change. Therefore, they can reposition their windows like suggest window, candidate window, information window, etc.

On the other hand, even if we put off flushing layout at focus, IME may request selection rectangles synchronously. Then, we need to flush layout synchronously in such case.

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/90fd055a4202 avoid flushing layout when notifying IME of focus events, r=masayuki