On Tue, Mar 25, 2014 at 6:18 AM, Botond Ballo <bba...@mozilla.com> wrote:
> Hello dev-platform,
>
> I recently fixed an APZ bug [1] that was caused by an IPDL message,
> PBrowser::UpdateFrame, being compressed when it shouldn't have been.
>
> I think the compression was correct back when we didn't have subframe
> scrolling, and so all messages pertained to the same frame, in which
> case it's appropriate to only act on the most recent message. With
> subframe scrolling, subsequent messages could apply to different
> frames, and dropping all but the most recent message could lead to a
> frame missing an important update.
>
> I fixed the bug by getting rid of the compression, but it occurred to
> me that if we had a mechanism for specifying that two messages
> applying to the same frame could be suppressed, but two messages
> applying to different frames could not, we'd probably want to use it.
>
> Is there any interest in adding such a mechanism to IPDL?
>
> Here's a straw man proposal for how it could work. Obviously, the
> syntax is not the interesting part and I'm happy to change it to
> whatever other syntax people might like:
>
> In addition to having a 'compress' attribute, have a 'compress_if'
> attribute that takes a function name as argument. Such a function
> would be expected to be provided in C++ code, to have 2*n arguments,
> where 'n' is the argument to the message, and to return 'bool'
> indicating whether or not two messages with those arguments should
> be suppressed.
>
> For example:
>
> RetType myMessage(Foo foo, Bar bar) compress_if(CompressMyMessage);
>
> And then one would provide:
>
> bool CompressMyMessage(const Foo& foo1, const Bar& bar1,
> const Foo& foo2, const Bar& bar2)
> {
> // whatever
> }
>
> which would decide whether or not myMessage(foo1, bar1) and
> myMessage(foo2, bar2) should be compressed.
>
> For example, for UpdateFrame we could do:
>
> UpdateFrame(FrameMetrics frame) compress_if(CompressUpdateFrame);
>
> and provide:
>
> bool CompressUpdateFrame(const FrameMetrics& frame1,
> const FrameMetrics& frame2)
> {
> // Two UpdateFrame messages can be compressed if they
> // apply to the same frame.
> return frame1.mScrollId == frame2.mScrollId;
> }
>
> I can think of at least one other use case in code that I'm
> currently working on: for bug 976605 [2], I'd find it
> convenient to conditionally compress PBrowser::RealTouchMoveEvent,
> to make sure that certain touch-move events (those with a
> particular flag set which is important for the child side to
> know about) are not compressed away.
>
> Presumably there will be other use cases in other protocols as well.
>
> Any thoughts about this / interest in this?
>
> Cheers,
> Botond
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=984673
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=976605
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
The name "compression" is unfortunate here. It took me some time to
understand what the real issue is.
That said, it seems like the "clean" architectural solution here is to
create a new actor for every combination of parameters that you do not
want to "compress" messages for, and leave as function parameters
every parameter that you do want to compress messages for. That may
not scale well, but in this case I think it would just mean creating
an actor for every (sub?)frame. Have we tried that? Is the
performance cost too high?
- Kyle
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
