1158111 - Add caching and control updating tab offset values in the child from the parent side

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Jim Mathies [:jimm]]

This is bad for performance of plugins, and generally bad for the browser too. We make this call for a number of plugin oriented calls that deal with coordinates, including the delivery of mouse events to windowless controls.

What we should do here:

1) cache the offset in the child
2) message the child from the parent when that value changes
 
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsPluginFrame.cpp#762

Because this is called so often, it ends up getting wrapped up in CPOW send aborts too landing at the top of our worst offenders list on a regular basis - 

Bug 1092216 Child abort on send signature breakdown:
----------------------------------------------------
25	27.2%	mozilla::dom::PBrowserChild::SendGetTabOffset(mozilla::gfx::IntPointTyped<mozilla::LayoutDevicePixel> *)
19	20.7%	mozilla::dom::TabChild::DoSendBlockingMessage(JSContext *,nsAString_internal const &,mozilla::dom::StructuredCloneData const &,JS::Handle<JSObject *>,nsIPrincipal *,nsTArray<nsString> *,bool)
18	19.6%	mozilla::dom::PContentChild::SendPRemoteSpellcheckEngineConstructor(mozilla::PRemoteSpellcheckEngineChild *)
14	15.2%	mozilla::dom::PStorageChild::SendPreload(nsCString const &,unsigned int const &,nsTArray<nsString> *,nsTArray<nsString> *,nsresult *)
12	13.0%	mozilla::dom::PBrowserChild::SendIsParentWindowMainWidgetVisible(bool *)
2	2.2%	mozilla::dom::PContentChild::SendGetRandomValues(unsigned int const &,nsTArray<unsigned char> *)
1	1.1%	mozilla::dom::PBrowserChild::SendGetDefaultScale(double *)
1	1.1%	mozilla::hal_sandbox::PHalChild::SendGetCurrentScreenConfiguration(mozilla::hal::ScreenConfiguration *)

Comment 1

[Jim Mathies [:jimm]]

Hey marcus, curious if you know of some sort of layout feature I can latch into for something like this. I'd like for the browser frame to subscribe to positional updates when the offset of the frame within its parent changes, and that would need to cascade up to the top level widget.

I was poking around in frame classes looking for something but didn't see much. This bug is a nice to have type bug since it would avoid a lot of sync child->parent calls but we could live without it if need be.

Comment 2

[Markus Stange [:mstange]]

(In reply to Jim Mathies [:jimm] from comment #1)
> Hey marcus, curious if you know of some sort of layout feature I can latch
> into for something like this. I'd like for the browser frame to subscribe to
> positional updates when the offset of the frame within its parent changes,
> and that would need to cascade up to the top level widget.

I don't know very much about reflow, but I don't think there's something exactly like that.
If a parent frame of the frame we're interested in gets moved during reflow, I don't know if we'll always descend into the moved frame's descendants. We probably don't.

There exist "reflow callbacks" which you can install on the presshell during reflow (PostReflowCallback), and those get called after reflow is finished, i.e. when all frames have their new positions + sizes. In the callback you could compare the new position of your frame with the old position and then handle changes. But a reflow callback is only installed for the current reflow, whereas you'll probably need something that stays registered for all reflows until you manually unregister it. I don't know if we have a good way to do that already.

Comment 3

[Jim Mathies [:jimm]]

(In reply to Markus Stange [:mstange] from comment #2)
> (In reply to Jim Mathies [:jimm] from comment #1)
> > Hey marcus, curious if you know of some sort of layout feature I can latch
> > into for something like this. I'd like for the browser frame to subscribe to
> > positional updates when the offset of the frame within its parent changes,
> > and that would need to cascade up to the top level widget.
> 
> I don't know very much about reflow, but I don't think there's something
> exactly like that.
> If a parent frame of the frame we're interested in gets moved during reflow,
> I don't know if we'll always descend into the moved frame's descendants. We
> probably don't.
> 
> There exist "reflow callbacks" which you can install on the presshell during
> reflow (PostReflowCallback), and those get called after reflow is finished,
> i.e. when all frames have their new positions + sizes. In the callback you
> could compare the new position of your frame with the old position and then
> handle changes. But a reflow callback is only installed for the current
> reflow, whereas you'll probably need something that stays registered for all
> reflows until you manually unregister it. I don't know if we have a good way
> to do that already.

Thanks, that got me looking in the right place. Turns out this was implemented over the summer while we were working on coordinate issue. :)

Comment 4

[Jim Mathies [:jimm]]

Attachment: patch

Swapping out one offset source for another.

Comment 5

[Jim Mathies [:jimm]]

Attachment: patch

merged to tip.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5247e8b25d9b

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5247e8b25d9b