1126090 - Bad event dispatch on engadget.com

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

With APZ enabled, when I visit engadget.com and hover over anything in the menu bar, I can still scroll the content underneath. With APZ disabled that doesn't happen.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

The InputQueue code for scroll events was never propagating nsEventStatus_eConsumeNoDefault back.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8556363 [details] [diff] [review]
fix

Review of attachment 8556363 [details] [diff] [review]:
-----------------------------------------------------------------

In general return values from AsyncPanZoomController::HandleInputEvent are not meant to be returned out of APZCTreeManager. Despite the fact that they are both nsEventStatus types, they are really incompatible. Can you explain more about the problem you're seeing here?

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix v2

Don't take ACTION_SCROLL if we expect the scroll animation to happen in the compositor.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8556735 [details] [diff] [review]
fix v2

Review of attachment 8556735 [details] [diff] [review]:
-----------------------------------------------------------------

You probably want to use "action" in the switch below?

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8556735 [details] [diff] [review]
fix v2

Hrm, you're right. I still had my original patch applied. With just this, things work, but the performance is really bad. So it must not be enough.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

Okay, this works after all. It just exacerbates a timing issue in the APZ smooth scrolling code where if a scroll animation completes very quickly then the velocity doesn't carry over to the next scroll event. That's a separate issue though.

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/integration/mozilla-inbound/rev/24fe1880f83b

Comment 8

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

Backed this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/645b6df65e31 for b2g failures:
https://treeherder.mozilla.org/logviewer.html#?job_id=6231262&repo=mozilla-inbound

Comment 9

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 1, don't double scroll

That failing test actually exposed quite a few problems. The first is that, not all wheel events will be handled by APZ, so the exact same check needs to be in EventStateManager as well.

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 2, clean up widget APZ dispatch

These aren't the best names, but whatever. The new method is virtual in nsIWidget so PuppetWidget can forward it to the parent process.

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 3, e10s fix

This makes synthesizeWheelEvent work with e10s+APZ. This event could *probably* be async since scrolls will be async anyway.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8560052 [details] [diff] [review]
part 1, don't double scroll

Review of attachment 8560052 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/events/EventStateManager.cpp
@@ +3016,5 @@
> +      // When APZ is enabled, the actual scroll animation might be handled by
> +      // the compositor.
> +      WheelPrefs::Action action = WheelPrefs::ACTION_NONE;
> +      if (!layers::APZCTreeManager::WillHandleWheelEvent(wheelEvent)) {
> +       action = WheelPrefs::GetInstance()->ComputeActionFor(wheelEvent);

indent

::: gfx/layers/apz/src/APZCTreeManager.cpp
@@ +859,5 @@
>  
> +bool
> +APZCTreeManager::WillHandleWheelEvent(WidgetWheelEvent* aEvent)
> +{
> +  return EventStateManager::WheelEventIsScrollAction(aEvent) &&

add a MOZ_ASSERT(aEvent)

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8560197 [details] [diff] [review]
part 2, clean up widget APZ dispatch

Review of attachment 8560197 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/nsBaseWidget.cpp
@@ +953,5 @@
>    }
>  }
>  
>  nsEventStatus
> +nsBaseWidget::ProcessAPZEvent(WidgetInputEvent* aEvent,

Rename this to ProcessUntransformedAPZEvent

@@ +979,5 @@
> +{
> +  if (mAPZC) {
> +      uint64_t inputBlockId = 0;
> +      ScrollableLayerGuid guid;
> +

This file appears to have 2-space indentation but this code does not

Comment 14

[Kartikaya Gupta (email:[e-mail])]

David, do you mind deferring landing this until the b2g stuff we need for 2.2 has landed? Specifically bug 930939 and bug 1127066, since they both touch some of this code and it'll be easier to uplift those to b2g37 if we don't have to rebase around this work.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8560052 [details] [diff] [review]
part 1, don't double scroll

>+APZCTreeManager::WillHandleWheelEvent(WidgetWheelEvent* aEvent)
>+{
>+  return EventStateManager::WheelEventIsScrollAction(aEvent) &&
>+         aEvent->deltaMode == nsIDOMWheelEvent::DOM_DELTA_LINE;

Not about this bug, but it is really not clear to me why only line scrolling is handled.
I'd actually expect apz to deal especially with pixel scrolling, and then possibly leave
line scrolling for ESM, since line scrolling depends on the target of the event.
(though, perhaps we know the line information somewhere in layer level too?)

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8560199 [details] [diff] [review]
part 3, e10s fix

I have no idea why prio(high).
(In other words, I don't understand what prio(high) means especially with sync)

Comment 17

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #14)
> David, do you mind deferring landing this until the b2g stuff we need for
> 2.2 has landed? Specifically bug 930939 and bug 1127066, since they both
> touch some of this code and it'll be easier to uplift those to b2g37 if we
> don't have to rebase around this work.

Missed this comment - but yeah, totally.

Comment 18

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 4, fix coordinates

Make sure we do the inverse of MapEventCoordinatesToChildProcess() when events originate in the other direction.

Comment 19

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8562337 [details] [diff] [review]
part 4, fix coordinates

Review of attachment 8562337 [details] [diff] [review]:
-----------------------------------------------------------------

Conceptually I agree this needs to be done, but I'm wondering if we can reuse more code. For one thing, RecvSynthesizedMouseWheelEvent already sets GetWidget() on the event as event.widget, so I feel like without too much trouble you should be able to use EventStateManager::GetChildProcessOffset directly (and then just negate the offset).

I might be misreading the code though, it's confusing and needs to be simplified. Does GetChildProcessOffset actually return an offset or an absolute value? It's odd because the current TabParent::MapEventCoordinatesForChildProcess assigns the offset directly to refPoint for non-touch events but then adds it to the refPoint for touch events. That's definitely bad code even if it happens to work out right, because it's relying on undocumented assumptions somewhere.

Comment 20

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #19)
> Comment on attachment 8562337 [details] [diff] [review]
> part 4, fix coordinates
> 
> Review of attachment 8562337 [details] [diff] [review]:
> -----------------------------------------------------------------
> I might be misreading the code though, it's confusing and needs to be
> simplified. Does GetChildProcessOffset actually return an offset or an
> absolute value? It's odd because the current
> TabParent::MapEventCoordinatesForChildProcess assigns the offset directly to
> refPoint for non-touch events but then adds it to the refPoint for touch
> events. That's definitely bad code even if it happens to work out right,
> because it's relying on undocumented assumptions somewhere.

GetChildProcessOffset returns an absolute value. Yes, it's confusing, I don't really know what's going on there. What is it supposed to do?

Comment 21

[Kartikaya Gupta (email:[e-mail])]

It's supposed to convert the event's coordinates from being in the parent process's widget space to the child process's widget space. On B2G for example there's a 30 pixel difference between the two in most apps, because the status bar at the top of the screen is 30 pixels tall and lives in the parent process. On desktop with e10s there is a similar delta from the chrome part which lives in the parent process.

Conceptually based on the name of the method I would expect that function to return the offset of child process's widget from the parent process's widget, and then the caller code would just add that to whatever event coordinates needed to be adjusted. However that's not what it does - it takes an event, and returns the value of the event's refPoint plus the offset into the child process. I think the reason that it works for touch events is because the touch event's refPoint is always zero, and the individual touch points are what matter. Therefore the call to ESM::GetChildProcessOffset for a *touch* event will return just the offset, which then gets added to the individual touch points mRefPoint.

Looking around, there is also TabParent::GetChildProcessOffset which does exactly what I would expect (although it returns an nsIntPoint instead of a LayoutDeviceIntPoint, but whatever). So we can use that instead - in fact, to fix the problem you're seeing, all you should need to do is make a one-line change to RecvSynthesizedMouseWheelEvent:

localEvent.refPoint -= LayoutDeviceIntPoint::FromUntyped(GetChildProcessOffset());

Nothing else should be needed. I can file another bug to get rid of EventStateManager::GetChildProcessOffset which doesn't really return an offset, and to make all code using switch over to using TabParent::GetChildProcessOffset.

Comment 22

[Kartikaya Gupta (email:[e-mail])]

see also https://bugzilla.mozilla.org/show_bug.cgi?id=1125040#c18

Comment 23

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 5, fix test

The failing test creates a scrollframe as part of its setup. The initial state of a scrollframe does not allow APZ scrolling until APZ acknowledges the initial scroll offset (which in the test is 0,0). I'm not sure I really like that, but I can't think of anything else in the meantime.

This patch just changes the test to wait for all paints to be flushed.

Comment 24

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 4, fix coordinates

Comment 25

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 6, fix single-line text boxes

Two more problems. The first is that on Desktop, text controls don't need focus to be scrolled. The second is that, single-line text controls can't be vertically scrolled with the mouse wheel even if their scrollport overflows the frame.

This patch sets a bit in FrameMetrics if the layer should not be vertically scrolled with the mouse wheel. APZ then treats any wheel-originated scroll in such layers as an overscroll.

This patch depends on bug 1102427, since the outer scrollable frames must be layerized for overscroll handoff to work.

Comment 26

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8566735 [details] [diff] [review]
part 6, fix single-line text boxes

Review of attachment 8566735 [details] [diff] [review]:
-----------------------------------------------------------------

I wonder if there's some way to roll this into the event-regions code. The behaviour here seems quite similar to doing touch-action:pan-x on the scrollable frame, except that it's more like wheel-action:pan-x since it's restricted to wheel events. Still we might be able to reuse some of that machinery to make this work. That being said that machinery isn't quite in place yet so maybe it's not worth waiting for that... I'm not really sure. Something to think about.

Comment 27

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #26)
> I wonder if there's some way to roll this into the event-regions code. The
> behaviour here seems quite similar to doing touch-action:pan-x on the
> scrollable frame, except that it's more like wheel-action:pan-x since it's
> restricted to wheel events. Still we might be able to reuse some of that
> machinery to make this work. That being said that machinery isn't quite in
> place yet so maybe it's not worth waiting for that... I'm not really sure.
> Something to think about.

What machinery is there already? Would we be able to handoff the scroll at the APZCTM level instead of at the overscroll decision? If so, adding another region type would be a lot cleaner.

Related, I'm not sure whether having both an x and y delta should turn into a y overscroll, or whether the y portion should just be dropped. I'm guessing we should drop the y portion. My trackpad doesn't seem to generate scrolls in both directions at once though.

Comment 28

[Kartikaya Gupta (email:[e-mail])]

(In reply to David Anderson [:dvander] from comment #27)
> What machinery is there already? Would we be able to handoff the scroll at
> the APZCTM level instead of at the overscroll decision? If so, adding
> another region type would be a lot cleaner.

The machinery for touch-action:pan-x would (once it's fully functional) cause TouchBlockState::TouchActionAllowsPanningXY() to return false while the innermost APZC was processing the input event (e.g. at [1]) and effectively prevent any vertical scrolling. I guess in your case you still want to propagate that scroll to the next APZC in the handoff chain which is not the case with touch-action. So maybe it's not that similar. :(

> Related, I'm not sure whether having both an x and y delta should turn into
> a y overscroll, or whether the y portion should just be dropped. I'm
> guessing we should drop the y portion. My trackpad doesn't seem to generate
> scrolls in both directions at once though.

I would say we should do a y overscroll. Consider the case where you have some magic trackpad that allows diagonal scrolls - it might be very hard for the user to actually scroll strictly vertically. Any amount of diagonal scroll would fall into this scenario. If we drop the y portion if there's an x component the user might stuck always panning horizontally inside the input box. Not sure if I misunderstood your question though...

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=d805db38cd5f#1787

Comment 29

[Botond Ballo [:botond]]

Comment on attachment 8566735 [details] [diff] [review]
part 6, fix single-line text boxes

Review of attachment 8566735 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/FrameMetrics.h
@@ +683,5 @@
>    // The value of GetLineScrollAmount(), for scroll frames.
>    LayoutDeviceIntSize mLineScrollAmount;
> +
> +  // Whether or not the frame can be vertically scrolled with a mouse wheel.
> +  bool mCannotVerticallyScrollWithWheel;

I prefer names that don't have negatives in them, e.g. mAllowVerticalWheelScroll.

::: gfx/layers/apz/src/APZCTreeManager.h
@@ +43,5 @@
> +enum ScrollSource {
> +  SCROLL_SOURCE_TOUCH,
> +  SCROLL_SOURCE_WHEEL,
> +  SCROLL_SOURCE_DOM    // scrollTo(), etc.
> +};

I'd prefer using an enum class (we can do that now!), and dropping the 'SCROLL_SOURCE_' prefix from the enumerator names.

@@ +361,5 @@
>     * Note: this should be used for panning only. For handing off overscroll for
>     *       a fling, use DispatchFling().
>     */
>    bool DispatchScroll(AsyncPanZoomController* aApzc,
> +                      ScrollSource aSource,

Make this a field of OverscrollHandoffState.

(You may have to move ScrollSource into APZUtils.h for header dependency reasons.)

::: gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ +780,5 @@
> +      // allowed here.
> +      overscroll.y = displacement.y;
> +    } else {
> +      mApzc.mY.AdjustDisplacement(displacement.y, adjustedOffset.y, overscroll.y);
> +    }

Maybe extract a helper function AsyncPanZoomController::AdjustYDisplacement that can be shared between here and AttemptScroll?

@@ +1039,5 @@
>      switch (panGestureInput.mType) {
>        case PanGestureInput::PANGESTURE_MAYSTART: rv = OnPanMayBegin(panGestureInput); break;
>        case PanGestureInput::PANGESTURE_CANCELLED: rv = OnPanCancelled(panGestureInput); break;
>        case PanGestureInput::PANGESTURE_START: rv = OnPanBegin(panGestureInput); break;
> +      case PanGestureInput::PANGESTURE_PAN: rv = OnPan(panGestureInput, SCROLL_SOURCE_TOUCH, true); break;

Are we considering trackpad to be "TOUCH" here?

::: layout/generic/nsGfxScrollFrame.cpp
@@ +1097,1 @@
>  #if defined(MOZ_B2G) || defined(MOZ_WIDGET_ANDROID)

Are you perhaps missing a changeset here? This #define is not on trunk, and I don't see the hunk that introduces it in any of the previous patches on this bug.

Comment 30

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Botond Ballo [:botond] from comment #29)
> ::: gfx/layers/apz/src/AsyncPanZoomController.cpp
> @@ +780,5 @@
> > +      // allowed here.
> > +      overscroll.y = displacement.y;
> > +    } else {
> > +      mApzc.mY.AdjustDisplacement(displacement.y, adjustedOffset.y, overscroll.y);
> > +    }
> 
> Maybe extract a helper function AsyncPanZoomController::AdjustYDisplacement
> that can be shared between here and AttemptScroll?
> 
> @@ +1039,5 @@
> >      switch (panGestureInput.mType) {
> >        case PanGestureInput::PANGESTURE_MAYSTART: rv = OnPanMayBegin(panGestureInput); break;
> >        case PanGestureInput::PANGESTURE_CANCELLED: rv = OnPanCancelled(panGestureInput); break;
> >        case PanGestureInput::PANGESTURE_START: rv = OnPanBegin(panGestureInput); break;
> > +      case PanGestureInput::PANGESTURE_PAN: rv = OnPan(panGestureInput, SCROLL_SOURCE_TOUCH, true); break;
> 
> Are we considering trackpad to be "TOUCH" here?

Yeah. The Mac touchpad sends these natively so we'll probably have to propagate the ScrollSource through the event itself, later on.

Comment 31

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 6, fix single-line text boxes v2

w/ comments addressed

Comment 32

[Botond Ballo [:botond]]

Comment on attachment 8567327 [details] [diff] [review]
part 6, fix single-line text boxes v2

Review of attachment 8567327 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. I'd like to have Timothy's feedback about the change to nsGfxScrollFrame.cpp.

::: gfx/layers/apz/src/APZUtils.h
@@ +26,5 @@
> +enum class ScrollSource {
> +  // scrollTo() or something similar.
> +  DOM,
> +
> +  // Touch-screen.

Please add "including trackpad".

::: gfx/layers/apz/src/Axis.cpp
@@ +128,5 @@
>      aDisplacementOut = 0;
>      return false;
>    }
> +  if (forceOverscroll) {
> +    aOverscrollAmountOut = aDisplacement;

Set aDisplacementOut to zero. It so happens that all existing callers pass in a zero-valued float, but we shouldn't rely on that.

::: layout/base/nsDisplayList.cpp
@@ +761,5 @@
>      LayoutDeviceIntSize lineScrollAmountInDevPixels =
>        LayoutDeviceIntSize::FromAppUnitsRounded(lineScrollAmount, presContext->AppUnitsPerDevPixel());
>      metrics.SetLineScrollAmount(lineScrollAmountInDevPixels);
> +
> +    if (EventStateManager::CanVerticallyScrollFrameWithWheel(aScrollFrame->GetParent())) {

Can aScrollFrame->GetParent() be null? EventStateManager::CanVerticallyScrollFrameWithWheel() doesn't check.

::: layout/generic/nsGfxScrollFrame.cpp
@@ +1095,5 @@
>                      && (styles.mHorizontal != NS_STYLE_OVERFLOW_HIDDEN);
> +
> +#if defined(MOZ_B2G) || defined(MOZ_WIDGET_ANDROID)
> +  // Mobile platforms need focus to scroll.
> +  bool canScroll = IsFocused(mOuter->GetContent());

This variable name and comment are a bit misleading. Given how we use them, I'd make it:

  // Mobile platforms need focus to scroll without scrollbars.
  bool canScrollWithoutScrollbars = ...;

@@ +1097,5 @@
> +#if defined(MOZ_B2G) || defined(MOZ_WIDGET_ANDROID)
> +  // Mobile platforms need focus to scroll.
> +  bool canScroll = IsFocused(mOuter->GetContent());
> +#else
> +  bool canScroll = true;

I'm not sure whether input fields are the only type of scrollable element that doesn't have scrollbars. If not, we may be making too many things scrollable here. I'd like to have Timothy's opinion on this.

Comment 33

[Timothy Nikkel (:tnikkel)]

(In reply to Botond Ballo [:botond] from comment #32)
> I'm not sure whether input fields are the only type of scrollable element
> that doesn't have scrollbars. If not, we may be making too many things
> scrollable here. I'd like to have Timothy's opinion on this.

ScrollFrameHelper::CreateAnonymousContent is what decides whether to create scrollbar frames or not. Places where it doesn't create scrollbar frames: text controls, print/print preview, svg documents being used as images and overflow hidden. We already handle overflow hidden in WantAsyncScroll. Probably doesn't matter for print/print preview. But inside svg image documents probably we should not consider things scrollable.

Comment 34

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8567327 [details] [diff] [review]
part 6, fix single-line text boxes v2

Assuming the svg images change is okay.

Comment 35

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: bug1126090-part7.patch

Another fix since some tests are still failing. If the scroll range of a frame is less than one device pixel, we're not supposed to scroll it.

Comment 36

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8570374 [details] [diff] [review]
bug1126090-part7.patch

Review of attachment 8570374 [details] [diff] [review]:
-----------------------------------------------------------------

This looks sane to me

::: layout/generic/nsGfxScrollFrame.cpp
@@ +1087,5 @@
>  
>  bool
>  ScrollFrameHelper::WantAsyncScroll() const
>  {
>    nsRect scrollRange = GetScrollRange();

don't need this local variable any more

Comment 37

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: follow-up fix

Fix failure on B2G. I don't like this very much. The local ref of mWidget->APZCTM() might be racy, if the APZCTM can be set to null on the widget.

Comment 38

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8571848 [details] [diff] [review]
follow-up fix

Review of attachment 8571848 [details] [diff] [review]:
-----------------------------------------------------------------

I also don't like this, but mostly because you haven't even described the problem this is trying to fix.

Comment 39

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: follow-up fix

B2G can't dispatch APZ events from the main thread.

Comment 40

[Kartikaya Gupta (email:[e-mail])]

Where are the wheel events coming from? B2G doesn't generate wheel events.

Comment 41

[Kartikaya Gupta (email:[e-mail])]

Ah I see, it's a test scenario and the previous patches on the bug pump the wheel event into the B2G widget code. This is getting pretty messy, and it would be nice to fork some of the older patches on this bug into new bugs and land them. Lemme think about this wheel event codepath though.

Comment 42

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8572096 [details] [diff] [review]
follow-up fix

Review of attachment 8572096 [details] [diff] [review]:
-----------------------------------------------------------------

As discussed on IRC I'd rather disable the test on B2G, as wheel input isn't a supported input method on B2G anyway.

Comment 43

[David Anderson [:dvander] - inactive, e-mail if emergency]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6bb1f6eaba4d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/2d15f031ead0
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/37077c295992
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e4d36ca729fd
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/574fe7c9b8e4
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c1c866236e5f
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/5921ef66656c

Comment 44

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6bb1f6eaba4d
https://hg.mozilla.org/mozilla-central/rev/2d15f031ead0
https://hg.mozilla.org/mozilla-central/rev/37077c295992
https://hg.mozilla.org/mozilla-central/rev/e4d36ca729fd
https://hg.mozilla.org/mozilla-central/rev/574fe7c9b8e4
https://hg.mozilla.org/mozilla-central/rev/c1c866236e5f
https://hg.mozilla.org/mozilla-central/rev/5921ef66656c

Comment 45

[Grant]

I think this bug broke scrolling completely on the 08/03 nightly. Scrolling is extremely slow with APZ enabled. going back to 07/03 fixes it