Loosely based on the approach of bug 1198966, mstange suggests that we should have the content thread initiate the start of an async scroll. The compositor could then take over the interaction. This means that the content thread needs to response to start the interaction but afterwards it's entirely async. This has the benefit that we don't need to duplicate some of the nsSliderFrame (scrollbar) logic in the compositor. However I imagine from this patch we could incrementally make the scroll start async if we wanted to duplicate that code.

Bug 1199885 - Let nsSliderFrame trigger an async APZ scroll

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Let nsSliderFrame trigger an async APZ scroll

This works much better than bug 1198966 as expected. This version of the patch works, but there's work remaining: - Properly calculate the scroll position (transform the event) - Handle vertical scrolling - Clamp the scrolling value

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Let nsSliderFrame trigger an async APZ scroll

https://reviewboard.mozilla.org/r/17707/#review15895 ::: layout/xul/nsSliderFrame.cpp:925 (Diff revision 3) > + tabChild->SendStartScrollbarDrag(PresContext()->PresShell()->GetPresShellId(), scrollTargetId, 0, mDragStart / 60); I think it would be better to route this through the widget rather than the tabchild directly. That way scrollbars in the parent process will also work the same way.

Thanks for the early feedback. I'll do that before posting it for review. It might take a few days at least. I only handle the trivial case ATM.

Here's a simple testcase where JS will move your scroll position each 2 seconds. Right now scroll bars don't take priority. Neither does it in Chrome/Safari. So I don't think we will implement giving the drag scroll priority.

Sounds good, that should keep the implementation simpler too.

So I need to know the size of the scroll thumb to compute the new scroll position. I will need to notify the APZC instance each time a transaction comes to tell the controller what the size of the scroll thumbs are. :(

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Let nsSliderFrame trigger an async APZ scroll

Mostly works now. The units conversion aren't 100% correct yet but almost. Need to test corner cases as well.

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Let nsSliderFrame trigger an async APZ scroll

Here's a good working snapshot. Deals well with offsets, content scales, CSS transforms and windows scrollbars. We decided we want to implemented input blocks.

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Let nsSliderFrame trigger an async APZ scroll. r?kats

This is behind a proper flag. I think it's a good idea to get this into the tree while we decide how to deal with the input block ordering.

I gave the input block discussion we had some thought and I think we can make it work without too much trouble. Basically this is what I envision: - We create a new input block type of "drag" - A "drag" block is created either when we get a mousedown or a drag event [1]. It would be terminated by a mouseup or non-drag event. So for an input sequence like "mousedown, drag, wheel, drag, mouseup", we would get a drag block for the first "mousedown, drag", then a wheel block for the wheel event, and a second (independent) drag block for "drag, mouseup". - This means that the main thread will need to send notifications to the APZ code not just on mousedown but also on drag events that start a new input block. This was the point at which we stopped our discussion - but handling this is actually pretty easy. The main thread gets an input block id with each event (see for example [2]) and any time that input block id changes we need to send a notification. So to handle this we just need add something like this at [3] and the equivalent TabChild mouse-handling code: } else if (aEvent is mousedown || aEvent is drag) { static uint64_t lastBlockId = 0; if (lastBlockId != aInputBlockId) { lastBlockId = aInputBlockId; APZCCallbackHelper::SendSetTargetAPZCNotification(...) // whatever other notifications } } ... Does that sound reasonable? I looked over the shape of your patch quickly and most of it looks pretty good, although there seems to be a bunch of stuff like StartScrollbarDragTimeout that I don't think we need if we use the input blocks properly. I'd also like to see that patch split up into smaller patches since it's kind of hard to review. I'll do a pass over it anyway. [1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/APZCTreeManager.cpp?rev=3906b2dab080#881 [2] http://mxr.mozilla.org/mozilla-central/source/widget/nsBaseWidget.cpp?rev=3906b2dab080#967 [3] http://mxr.mozilla.org/mozilla-central/source/widget/nsBaseWidget.cpp?rev=3906b2dab080#1002

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats https://reviewboard.mozilla.org/r/17707/#review16901 This is a really big patch and introduces a pretty large change. Thanks for taking it on! While I do want to see this go forward I feel like it's going to take a bunch more work and it's not really something we can just whip up in a day. Honestly if you have spare cycles to work on APZ stuff I'd rather see that effort go towards all the existing bugs we have with wheel scrolling before trying to introduce a whole new set of bugs with scrollbar dragging. I realize scrollbar dragging is just as valid a use case as wheel but looking at the size and complexity of this and the number of edge cases I can think of, I'd much rather wait on this until we have all the other major bugs hammered out. If you want to continue working on this though at the very least I'd like to see this patch split up into a queue of smaller patches. I think the first few patches would be (1) adding the new class to InputData.h, (2) Updating the APZTreeManager and InputQueue to create proper drag blocks out of the relevant mouse events, even though the blocks will have no special behaviour (i.e. AsyncPanZoomController will just ignore them), (3) hook up the nsSliderFrame machinery so that it notifies APZTreeManager as whether or not the drag input block should be handled in APZ. ::: dom/ipc/PBrowser.ipdl:565 (Diff revision 6) > + async StartScrollbarDragTimeout(uint64_t aBlockId); If we use input blocks properly I don't think we will need the StartScrollbarDragTimeout function at all ::: gfx/layers/apz/src/AsyncDragMetrics.h:11 (Diff revision 6) > +#include "mozilla/TimeStamp.h" I don't think you need this or the forward-declaration for TimeStamp (and certainly not both) ::: gfx/layers/apz/src/AsyncDragMetrics.h:34 (Diff revision 6) > + AsyncDragMetrics() Not sure why you have this default constructor, it leaves stuff uninitialized and so I'd get rid of it if possible. ::: gfx/layers/apz/src/AsyncPanZoomController.h:394 (Diff revision 6) > + void ScheduleComposite(); Why did you move this declaration? ::: gfx/layers/apz/src/InputQueue.cpp:196 (Diff revision 6) > + prevBlock->ShareTarget(block); This won't always work, because the old mouse block can get dropped from the queue after is done being processed. That's the main reason I think it is better to just start a new independent block with its own notifications from the main thread. ::: gfx/layers/moz.build:10 (Diff revision 6) > +CXXFLAGS += ['-O0', '-g'] Drop this ::: widget/InputData.h:255 (Diff revision 6) > + MOUSE_MOVE, We'll need a MOUSE_DRAG type as well ::: widget/InputData.h:285 (Diff revision 6) > + ScreenPoint mOrigin; Will need a ParentLayerPoint mLocalOrigin for the untransformed coordinate

https://reviewboard.mozilla.org/r/17707/#review16901 I knew it would be significantly longer when we stopped doing the first trivial approach. I agree it will take longer than a day, well over a week of work left in fact but I'm happy to do it. In the mean time I'd like to get this landed behind a preference. This touches a lot of the tree and will rot if it's not checked in. As far I know the approach is good so there shouldn't be any adverse effects from checking this in under a flag. I can understand not being in a rush to turn this on and have more bugs to deal with so that can wait. If your concern is review bandwidth then let me know. Last I synced up with mstange he was building on a patch queue to address several issue. There wasn't really anything I could jump into effectively without contenting. If there's bugs I can help with now effectively let me know. I was also asked to look into painting performance and after Talos and fixing the DP perf issues this was the next low hanging fruit. While it's not simple, it's not that hard either and is a fairly big win. I'm happy to gate turning this on against a high quality bar. I can split out the parts if you'd like. The partition is mostly at a file level so I didn't think it was neccessary. > Not sure why you have this default constructor, it leaves stuff uninitialized and so I'd get rid of it if possible. It's for IPC. I though I left a comment in the code to point that out. I'll use friend and make it private. > Why did you move this declaration? I think I can clean this up now.

(In reply to Benoit Girard (:BenWa) from comment #18) > I knew it would be significantly longer when we stopped doing the first > trivial approach. I agree it will take longer than a day, well over a week > of work left in fact but I'm happy to do it. In the mean time I'd like to > get this landed behind a preference. This touches a lot of the tree and will > rot if it's not checked in. As far I know the approach is good so there > shouldn't be any adverse effects from checking this in under a flag. I can > understand not being in a rush to turn this on and have more bugs to deal > with so that can wait. If your concern is review bandwidth then let me know. Ok, fair enough. Let's clean it up and get it landed. I would still like to bring up the quality of the patch somewhat, and avoid landing code that we will need to rip out later when we do proper input blocks - there's no point to landing that because it will just clutter the code and make other changes harder, and even if it rots it doesn't matter because we won't be needing it later. Are you ok with that? > Last I synced up with mstange he was building on a patch queue to address > several issue. There wasn't really anything I could jump into effectively > without contenting. If there's bugs I can help with now effectively let me > know. Something like bug 1189565 would be a really great one to pick up since it's not tangled with anything else anybody is working on. > I can split out the parts if you'd like. The partition is mostly at a file > level so I didn't think it was neccessary. Yes please. MozReview makes it really annoying to review a monolithic patch because jumping around between specific files is hard (I filed bug 1203969 for this as I was looking at your patch). Also if you split it up we can get stuff landed incrementally to prevent rot. Thanks!

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #19) > (In reply to Benoit Girard (:BenWa) from comment #18) > > I knew it would be significantly longer when we stopped doing the first > > trivial approach. I agree it will take longer than a day, well over a week > > of work left in fact but I'm happy to do it. In the mean time I'd like to > > get this landed behind a preference. This touches a lot of the tree and will > > rot if it's not checked in. As far I know the approach is good so there > > shouldn't be any adverse effects from checking this in under a flag. I can > > understand not being in a rush to turn this on and have more bugs to deal > > with so that can wait. If your concern is review bandwidth then let me know. > > Ok, fair enough. Let's clean it up and get it landed. I would still like to > bring up the quality of the patch somewhat, and avoid landing code that we > will need to rip out later when we do proper input blocks - there's no point > to landing that because it will just clutter the code and make other changes > harder, and even if it rots it doesn't matter because we won't be needing it > later. Are you ok with that? > Yes absolutely. My intention here isn't to strong arm you to accept a bad patch under the excuse 'it's behind a preference'. Let's make sure we're happy with the patch. I'd like to make sure everything about this patch, expect the input block handling, to be great. If you want I can remove that portion of the code entirely from the patch since that is tangled. > > Last I synced up with mstange he was building on a patch queue to address > > several issue. There wasn't really anything I could jump into effectively > > without contenting. If there's bugs I can help with now effectively let me > > know. > > Something like bug 1189565 would be a really great one to pick up since it's > not tangled with anything else anybody is working on. > I'll take a look. I think this was one of the bugs I discussed with mstange but it was landing when I last looked. > > I can split out the parts if you'd like. The partition is mostly at a file > > level so I didn't think it was neccessary. > > Yes please. MozReview makes it really annoying to review a monolithic patch > because jumping around between specific files is hard (I filed bug 1203969 > for this as I was looking at your patch). Also if you split it up we can get > stuff landed incrementally to prevent rot. Thanks! Will do.

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Part 1: Add MouseInput InputData. r?kats

Bug 1199885 - Part 2: Add AsyncDragMetrics. r?kats

Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r?kats

Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r?kats

Bug 1199885 - Part 5: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats

Alright here's a proper patch queue. I could split part 5 more but IMO this block really needs to be reviewed together. Note that I didn't restore my previous intermediate 'works but gets input blocks wrong' state, now this patch doesn't work at all but it will all be behind a preference. We've got a plan for input blocks so I'd rather just work on that. So ideally we should be fully satisfied with this queue, minus it not dealing with input blocks.

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats https://reviewboard.mozilla.org/r/17707/#review16931 ::: widget/InputData.h:254 (Diff revision 7) > + enum MouseType { nit: brace on new line ::: widget/InputData.cpp:63 (Diff revision 7) > + mType = MOUSE_DRAG_START; missing break; ::: widget/InputData.cpp:64 (Diff revision 7) > + case eDragOver: From my reading of https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Drag_and_drop#events I think you want eDragEnd instead of eDragOver, and you want to map eDrag to something like MOUSE_DRAG. ::: widget/InputData.cpp:68 (Diff revision 7) > + MOZ_ASSERT_UNREACHABLE("Mouse event type not supported"); Crashing here seems not that great, but I guess it depends on what we do at the call sites of this function. Once I look at the rest of the queue I might come back to this.

Comment on attachment 8660105 [details] MozReview Request: Bug 1199885 - Part 2: Add AsyncDragMetrics. r=kats https://reviewboard.mozilla.org/r/19063/#review16933

Comment on attachment 8660106 [details] MozReview Request: Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r=kats https://reviewboard.mozilla.org/r/19065/#review16935 ::: toolkit/locales/en-US/chrome/global/aboutSupport.properties:107 (Diff revision 1) > +dragEnabled = drag input enabled Maybe "scrollbar drag enabled" would be more meaningful since this is a user-visible string?

Comment on attachment 8660107 [details] MozReview Request: Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r=kats https://reviewboard.mozilla.org/r/19067/#review16937 ::: gfx/layers/apz/src/HitTestingTreeNode.h:16 (Diff revision 1) > +#include "Layers.h" Can we get away with forward-declarations instead of including Layers.h? It's just such a big header to be including here :/ If we do need to include it, please move it up to between FrameMetrics.h and Matrix.h so that it's alphabetical. ::: gfx/layers/apz/src/HitTestingTreeNode.h:95 (Diff revision 1) > + FrameMetrics::ViewID GetScrollViewId() const { return mScrollViewId; } In general I prefer if the body of even simple getters are in the .cpp file, because it's easier to set breakpoints and inserting printfs doesn't cause a massive recompile. In this case I would also get rid of the GetScrollViewId and GetScrollDirection getters entirely since they don't need to be exposed and break encapsulation. Instead add a |bool MatchesScrollDragMetrics(const AsyncDragMetrics& aDragMetrics) const;| which you can call from FindScrollNode(...) The GetScrollSize() can stay, but move the body into the .cpp file.

Comment on attachment 8660108 [details] MozReview Request: Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r?kats https://reviewboard.mozilla.org/r/19073/#review16941 I have to head out soon - couple of comments below but if you're splitting this into smaller patches I'd rather wait for those. If you end up not doing that just re-flag me for review on the bugzilla attachment and I'll pick this up on Monday. ::: layout/base/Units.h:179 (Diff revision 1) > + static CSSIntRect FromAppUnits(const nsIntRect& aRect) { nsIntRect is usually not in app units, it's usually LayoutDevicePixel units. Please remove this and see my other comment at the call site. ::: layout/xul/nsSliderFrame.cpp:911 (Diff revision 1) > + nsIntRect sliderTrack(GetRect().x - scrollbarBox->GetRect().x, This should be an nsRect. In fact I would rewrite this as nsRect sliderTrack = GetRect() - scrollbarBox.GetPosition();

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #30) > Comment on attachment 8660107 [details] > MozReview Request: Bug 1199885 - Part 4: Let the hit testing tree know about > the scroll thumb. r?kats > > https://reviewboard.mozilla.org/r/19067/#review16937 > > ::: gfx/layers/apz/src/HitTestingTreeNode.h:16 > (Diff revision 1) > > +#include "Layers.h" > > Can we get away with forward-declarations instead of including Layers.h? > It's just such a big header to be including here :/ > > If we do need to include it, please move it up to between FrameMetrics.h and > Matrix.h so that it's alphabetical. It's not ideal I agree but we can't forward declare ScrollDirection otherwise. We'd have to move ScrollDirection outside of Layers.h but I don't think it's worth paying that cost just to minimize includes.

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Part 1: Add MouseInput InputData. r?kats

Comment on attachment 8660105 [details] MozReview Request: Bug 1199885 - Part 2: Add AsyncDragMetrics. r=kats Bug 1199885 - Part 2: Add AsyncDragMetrics. r=kats

Comment on attachment 8660106 [details] MozReview Request: Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r=kats Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r=kats

Comment on attachment 8660107 [details] MozReview Request: Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r=kats Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r?kats

Comment on attachment 8660108 [details] MozReview Request: Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r?kats Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r?kats

Bug 1199885 - Part 6: Let events know which input blocks to belong to confirm them. r?kats

Bug 1199885 - Part 7: Add ScrollTo to FrameMetrics for absolute scrolling. r?kats

Bug 1199885 - Part 8: Add FindScrollNode to locate the scrollbar thumb. r?kats

Bug 1199885 - Part 9: Let APZC handle the drag events. r?kats

Bug 1199885 - Part 10: Add APZTeeManager API to start an async scroll. r?kats

Bug 1199885 - Part 11: Let APZCCallbackHelper dispatch StartScrollbarDrag to the APZ controller thread. r?kats

Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r?kats

Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats https://reviewboard.mozilla.org/r/17707/#review17063

https://reviewboard.mozilla.org/r/19065/#review17065 ::: toolkit/locales/en-US/chrome/global/aboutSupport.properties:107 (Diff revisions 1 - 2) > -dragEnabled = drag input enabled > +dragEnabled = scroolbar drag enabled typo: s/scroolbar/scrollbar/

Comment on attachment 8660107 [details] MozReview Request: Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r=kats https://reviewboard.mozilla.org/r/19067/#review17067 ::: gfx/layers/apz/src/HitTestingTreeNode.h:16 (Diff revision 2) > +#include "Layers.h" Please move this #include up to between FrameMetrics.h and Matrix.h to keep it alphabetical

Comment on attachment 8660108 [details] MozReview Request: Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r?kats https://reviewboard.mozilla.org/r/19073/#review17069

Comment on attachment 8660859 [details] MozReview Request: Bug 1199885 - Part 9: Let APZC handle the drag events. r=kats https://reviewboard.mozilla.org/r/19169/#review17071 ::: gfx/layers/FrameMetrics.h:201 (Diff revision 1) > + void ScrollTo(const CSSPoint& aPoint) We already have SetScrollOffset which does the exact same thing. Either use that or file a follow-up to remove that one and convert all the uses to ScrollTo

Comment on attachment 8660860 [details] MozReview Request: Bug 1199885 - Part 9.5: Make the mouse events APZC aware. r?kats https://reviewboard.mozilla.org/r/19171/#review17073 ::: gfx/layers/apz/src/APZCTreeManager.h:303 (Diff revision 1) > + HitTestingTreeNode* FindScrollNode(const AsyncDragMetrics& aDragMetrics); This can't be made public as-is. Either it needs to return a nsRefPtr<HitTestingTreeNode>, and the implementation has to acquire the tree lock, or you have to make it private. ::: gfx/layers/apz/src/APZCTreeManager.cpp:1513 (Diff revision 1) > + if (((node->GetScrollDirection() == Layer::HORIZONTAL && aScrollDir == AsyncDragMetrics::HORIZONTAL) || This doesn't look like it's been rebased properly, need to use node->MatchesScrollDragMetrics instead. It will probably also change the signature of this function since you need to pass in the AsyncDragMetrics object itself.

Comment on attachment 8660858 [details] MozReview Request: Bug 1199885 - Part 8: Add FindScrollNode to locate the scrollbar thumb. r?kats https://reviewboard.mozilla.org/r/19167/#review17077 This adds an extra field to every single WidgetEvent when you only really need it in a small subset of cases. Instead of doing this, you should: (1) route mouse events through TabParent/TabChild in the same way that mousewheel events are routed. Specifically, in TabParent::SendRealMouseEvent call ApzAwareEventRoutingToChild to get the guid/blockId and send that to the child process (see how it's done in SendMouseWheelEvent). Calling ApzAwareEventRoutingToChild is something you'll need to do in the future for this mouse events anyway, so might as well do it now as it helps solve this problem. (2) Then in TabChild::RecvRealMouseButtonEvent, which actually dispatches the event to content (and which will invoke nsSliderFrame::HandleEvent), create another InputAPZContext object on the stack (before the call APZCCallbackHelper::DispatchWidgetEvent). InputAPZContext is a RAII helper which will provide access to the input block, so the code in nsSliderFrame::HandleEvent can access it via InputAPZContext::GetInputBlockId(). Doing these two things will allow the nsSliderFrame to know the input block id of the current mouse event, regardless of whether the code is in the child or parent process. (For the parent process, the InputAPZContext on the stack will be the one created in nsBaseWidget::ProcessUntransformedAPZEvent). And you won't need to add the extra field to WidgetEvent.

Comment on attachment 8660865 [details] MozReview Request: Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats https://reviewboard.mozilla.org/r/19181/#review17085 ::: layout/xul/nsSliderFrame.cpp:899 (Diff revision 1) > +bool unused return value, did you intend to use that somehow? ::: layout/xul/nsSliderFrame.cpp:914 (Diff revision 1) > + nsIContent* scrollableContent = GetScrollbar()->GetParent()->GetContent(); Can GetScrollbar()->GetParent() return null here? I assume yes; you may want to null-check for that also. ::: layout/xul/nsSliderFrame.cpp:920 (Diff revision 1) > + bool hasAPZView = (scrollTargetId != layers::FrameMetrics::NULL_SCROLL_ID); hasAPZView will always be true, because FindOrCreateIDFor will never return NULL_SCROLL_ID. I think what you meant to do was use FindIDFor instead of FindOrCreateIDFor

Comment on attachment 8660864 [details] MozReview Request: Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r?kats https://reviewboard.mozilla.org/r/19179/#review17087 ::: dom/ipc/TabParent.cpp:2954 (Diff revision 1) > +TabParent::RecvStartScrollbarDrag(const AsyncDragMetrics& aDragMetrics) Move the meat of this function into RenderFrameParent so you don't need to expose the APZCTreeManager as public. Also this function is busted because it does the nullcheck for GetRenderFrame() after it calls GetApzcTreeManager on it... This function should just look like: if (RenderFrameParent* rfp = GetRenderFrame()) { rfp->StartScrollbarDrag(aDragMetrics); } return true; ::: widget/nsBaseWidget.cpp:1683 (Diff revision 1) > +nsBaseWidget::StartAsyncScrollbarDrag(const AsyncDragMetrics& aDragMetrics) Add a |if (!AsyncPanZoomEnabled()) return| in here somewhere (preferably right at the top of the function). It's possible to have widgets which don't have an APZ instance even if the global APZ pref is enabled (non-e10s widgets for example). In those cases mAPZC will be null.

Comment on attachment 8660863 [details] MozReview Request: Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats https://reviewboard.mozilla.org/r/19177/#review17089 ::: gfx/layers/apz/util/APZCCallbackHelper.cpp:847 (Diff revision 1) > +class StartScrollbarDragTask : public Task You don't need this task at all. You can inline this trivially into the two call sites that call APZCCallbackHelper::StartScrollbarDrag: APZThreadUtils::RunOnControllerThread(NewRunnableMethod(apzctm, &APZCTreeManager::StartScrollbarDrag, aGuid, aDragMetrics));

Comment on attachment 8660862 [details] MozReview Request: Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r?kats https://reviewboard.mozilla.org/r/19175/#review17095 ::: gfx/layers/apz/src/APZCTreeManager.h:544 (Diff revision 1) > + AsyncDragMetrics mDragMetrics; These two fields aren't used yet so we can take them out for now. ::: gfx/layers/apz/src/APZCTreeManager.cpp:330 (Diff revision 1) > + // TODO Confirm the input block FWIW yes, this is pretty much what I had in mind for the architecture ::: gfx/layers/apz/src/APZCTreeManager.cpp:658 (Diff revision 1) > + mRootNode->GetApzc(), If you hard-code the root node's APZC here, it won't work for subframes. (Or worse, dragging a subframe's scrollbar will end up scrolling the root document). You should call GetTargetAPZC to get an APZC, much the way APZCTreeManager::ProcessEvent does it. But see my next comment. ::: gfx/layers/apz/src/APZCTreeManager.cpp:662 (Diff revision 1) > + break; Hmm. You'll need apply an APZC's untransform to mouseInput.mOrigin after this call to ReceiveInputEvent, so that when gecko sees this event it has the right coordinates. However dragging on a scrollbar probably behaves differently from the other input types in that the gecko coordinates are the same as the screen coordinates even though the async transform on the APZC is changing. I'm not really sure at this point and it'll take some though to figure out properly. For now this code won't get hit unless the pref is enabled, so we can just stick in a TODO and leave it as-is. (This goes for the previous comment as well) ::: gfx/layers/apz/src/APZCTreeManager.cpp:984 (Diff revision 1) > + uint64_t outInputBlockId; If you get rid of the inputBlockId member on WidgetEvent, you can get rid of this also, and just pass aOutInputBlockId directly to ReceiveInputEvent. ::: gfx/layers/apz/src/APZCTreeManager.cpp:986 (Diff revision 1) > + // REVIEW: aEvent.refPoint type safety? Assigning the refPoint to mOrigin here is (I believe) correct, although perhaps it would be cleaner to do it as part of the MouseInput constructor. However, *after* the call to ReceiveInputEvent, you need to overwrite aEvent.refPoint with the input.mOrigin value, because ReceiveInputEvent is supposed to do an in-place modification on that. That isn't happening yet (see my previous comments) but I'd like to see the code here updated to assume it is happening, so that we don't forget to do it later. See ProcessWheelEvent for an example of how.

Comment on attachment 8660861 [details] MozReview Request: Bug 1199885 - Part 10: Add APZTeeManager API to start an async scroll. r?kats https://reviewboard.mozilla.org/r/19173/#review17083 This code is mostly not invoked at the moment so I didn't go over it detail. We can land it as-is for now and revisit it later. ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:68 (Diff revision 1) > +#include "HitTestingTreeNode.h" Move up to alphabetize ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:993 (Diff revision 1) > + ParentLayerPoint geckoPoint = TransformTo<ParentLayerPixel>(transformToGecko, point); So what you're doing here will give you the right math but is labelled wrong. GetScreenToApzcTransform(this) will return a screen-to-apzc transform, so calling it transformToGecko is incorrect. Calling this variable "geckoPoint" is also incorrect. For other input events we have a TransformToLocal function on the input object (see for example ScrollWheelInput::TransformToLocal) which does this conversion from screen space to APZC space and puts it in the "local" field of the input event. We can follow the same pattern here.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #56) > ::: gfx/layers/apz/src/APZCTreeManager.cpp:658 > (Diff revision 1) > > + mRootNode->GetApzc(), > > If you hard-code the root node's APZC here, it won't work for subframes. (Or > worse, dragging a subframe's scrollbar will end up scrolling the root > document). You should call GetTargetAPZC to get an APZC, much the way > APZCTreeManager::ProcessEvent does it. But see my next comment. > This part here is not clear. If I call GetTargetAPZC I'll get an approximate APZC for the event which may not be the one that the content confirms. It feels like I should leave the APZC as null until the content can confirm but this would require some refactoring of ReceiveInputEvent. This is what I had tried first but I didn't want to start refactoring this on a hunch and I was not sure how deep the assumption that an input event always has an APZC. (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #53) > ::: layout/xul/nsSliderFrame.cpp:899 > (Diff revision 1) > > +bool > > unused return value, did you intend to use that somehow? I'm not using it but it felt useful to include. I did use it a bit for testing. I can remove it if we don't want it but I didn't think there was any harm in leaving it.

(In reply to Benoit Girard (:BenWa) from comment #58) > This part here is not clear. If I call GetTargetAPZC I'll get an approximate > APZC for the event which may not be the one that the content confirms. That's correct, and that should be fine. > It feels like I should leave the APZC as null until the content can confirm > but this would require some refactoring of ReceiveInputEvent. This is what I > had tried first but I didn't want to start refactoring this on a hunch and I > was not sure how deep the assumption that an input event always has an APZC. The assumption that an input block has a non-null APZC on creation is pretty deeply baked in. The APZC can be changed to null later if content says so. > (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #53) > > ::: layout/xul/nsSliderFrame.cpp:899 > > (Diff revision 1) > > > +bool > > > > unused return value, did you intend to use that somehow? > > I'm not using it but it felt useful to include. I did use it a bit for > testing. I can remove it if we don't want it but I didn't think there was > any harm in leaving it. Ok, that's fine.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #59) > (In reply to Benoit Girard (:BenWa) from comment #58) > > This part here is not clear. If I call GetTargetAPZC I'll get an approximate > > APZC for the event which may not be the one that the content confirms. > > That's correct, and that should be fine. > Then how good does the 'approximate guess' have to be then? Always picking mRootNode is an approximate guess for some very weak accuracy / high precision criterion. What property does GetTargetAPZC provide that we need? Or will it be incorrect sometimes too if the wrong subframe is picked?

The guess is used to compute the async untransform for the input event, so that it gets converted from screen space into the layout-device space that Gecko is expecting. It's true that in the cases we guess wrong this untransform will be wrong but in practice the guess is "good enough" that errors are not noticeable. If the guess is less accurate, such as with always using the root node, the errors may become more noticeable.

Ok thanks!

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Part 1: Add MouseInput InputData. r=kats

Comment on attachment 8660105 [details] MozReview Request: Bug 1199885 - Part 2: Add AsyncDragMetrics. r=kats Bug 1199885 - Part 2: Add AsyncDragMetrics. r=kats

Comment on attachment 8660106 [details] MozReview Request: Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r=kats Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r=kats

Comment on attachment 8660107 [details] MozReview Request: Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r=kats Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r=kats

Comment on attachment 8660108 [details] MozReview Request: Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r?kats Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r?kats

Comment on attachment 8660858 [details] MozReview Request: Bug 1199885 - Part 8: Add FindScrollNode to locate the scrollbar thumb. r?kats Bug 1199885 - Part 8: Add FindScrollNode to locate the scrollbar thumb. r?kats

Comment on attachment 8660859 [details] MozReview Request: Bug 1199885 - Part 9: Let APZC handle the drag events. r=kats Bug 1199885 - Part 9: Let APZC handle the drag events. r=kats

Comment on attachment 8660860 [details] MozReview Request: Bug 1199885 - Part 9.5: Make the mouse events APZC aware. r?kats Bug 1199885 - Part 9.5: Make the mouse events APZC aware. r?kats

Comment on attachment 8660861 [details] MozReview Request: Bug 1199885 - Part 10: Add APZTeeManager API to start an async scroll. r?kats Bug 1199885 - Part 10: Add APZTeeManager API to start an async scroll. r?kats

Comment on attachment 8660862 [details] MozReview Request: Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r?kats Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r?kats

Comment on attachment 8660863 [details] MozReview Request: Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats

Part 6 and 7 has been deleted, part 9.5 has been introduced between 9 and 10.

Comment on attachment 8660858 [details] MozReview Request: Bug 1199885 - Part 8: Add FindScrollNode to locate the scrollbar thumb. r?kats https://reviewboard.mozilla.org/r/19167/#review17837 ::: gfx/layers/apz/src/APZCTreeManager.h:19 (Diff revision 2) > +#include "mozilla/layers/AsyncDragMetrics.h" // for AsyncDragMetrics Shouldn't need this include here

Comment on attachment 8660859 [details] MozReview Request: Bug 1199885 - Part 9: Let APZC handle the drag events. r=kats https://reviewboard.mozilla.org/r/19169/#review17839 ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1010 (Diff revision 2) > + GetFrameMetrics().GetViewport().height; GetViewport() is the wrong thing to be using here, that's the CSS viewport. The composition bounds is what you want. Add a TODO for it. ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1013 (Diff revision 2) > + scrollPosition = std::max(0.f, scrollPosition); Instead of 0 we should be using the GetScrollableRect()'s left/top value. For RTL pages the leftmost scroll position may be nonzero. Add a TODO for this. ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1016 (Diff revision 2) > + mFrameMetrics.SetScrollOffset(CSSPoint(0, scrollPosition)); Half of this code is axis-independent and the other half assumes the y-axis. Please add a TODO so that this is corrected before we start using this code. ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1077 (Diff revision 2) > + if (!scrollInput.TransformToLocal(aTransformToApzc)) { nit: trailing whitespace

Comment on attachment 8660860 [details] MozReview Request: Bug 1199885 - Part 9.5: Make the mouse events APZC aware. r?kats https://reviewboard.mozilla.org/r/19171/#review17841 ::: dom/ipc/TabParent.cpp:1462 (Diff revision 2) > + event.refPoint += GetChildProcessOffset(); Remove this line; it is already done earlier in the function and doing it twice will result in incorrect mouse events. ::: dom/ipc/TabParent.cpp:3556 (Diff revision 2) > + remove spurious blank lines

Comment on attachment 8660862 [details] MozReview Request: Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r?kats https://reviewboard.mozilla.org/r/19175/#review17843 ::: dom/ipc/TabParent.cpp:2961 (Diff revision 2) > + rfp->StartScrollbarDrag(aDragMetrics); 2-space indent ::: layout/ipc/RenderFrameParent.h:108 (Diff revision 2) > + layers::APZCTreeManager* GetApzcTreeManager(); Move this back to where it was in the protected section ::: layout/ipc/RenderFrameParent.cpp:592 (Diff revision 2) > + layers::APZCTreeManager* apzctm = GetApzcTreeManager(); GetApzcTreeManager() could return null, please null check like all the other functions

Comment on attachment 8660863 [details] MozReview Request: Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats https://reviewboard.mozilla.org/r/19177/#review17845 ::: layout/xul/nsSliderFrame.cpp:910 (Diff revision 2) > + scrollbar = GetContentOfBox(scrollbarBox); combine this assignment into the declaration on the previous line ::: layout/xul/nsSliderFrame.cpp:913 (Diff revision 2) > + nsRect sliderTrack = GetRect() - scrollbarBox->GetPosition(); Move these declarations down to after the early returns, no point computing these if we're not going to use them. ::: layout/xul/nsSliderFrame.cpp:929 (Diff revision 2) > + uint64_t inputblockId = InputAPZContext::GetInputBlockId(); Move this down to past the early return as well. ::: layout/xul/nsSliderFrame.cpp:937 (Diff revision 2) > + NSAppUnitsToIntPixels(mDragStart, This makes me think the AsyncDragMetrics should store this value as a CSSIntCoord rather than an int32_t, so that we have it strongly typed. Stick in a TODO for it or something.

https://reviewboard.mozilla.org/r/19169/#review17839 > GetViewport() is the wrong thing to be using here, that's the CSS viewport. The composition bounds is what you want. Add a TODO for it. fixed instead of adding a todo. > Instead of 0 we should be using the GetScrollableRect()'s left/top value. For RTL pages the leftmost scroll position may be nonzero. Add a TODO for this. fixed instead > Half of this code is axis-independent and the other half assumes the y-axis. Please add a TODO so that this is corrected before we start using this code. fixed instead

Comment on attachment 8654469 [details] MozReview Request: Bug 1199885 - Part 1: Add MouseInput InputData. r=kats Bug 1199885 - Part 1: Add MouseInput InputData. r=kats

Comment on attachment 8660105 [details] MozReview Request: Bug 1199885 - Part 2: Add AsyncDragMetrics. r=kats Bug 1199885 - Part 2: Add AsyncDragMetrics. r=kats

Comment on attachment 8660106 [details] MozReview Request: Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r=kats Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r=kats

Comment on attachment 8660107 [details] MozReview Request: Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r=kats Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r=kats

Comment on attachment 8660108 [details] MozReview Request: Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r?kats Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r?kats

Comment on attachment 8660858 [details] MozReview Request: Bug 1199885 - Part 8: Add FindScrollNode to locate the scrollbar thumb. r?kats Bug 1199885 - Part 8: Add FindScrollNode to locate the scrollbar thumb. r?kats

Comment on attachment 8660859 [details] MozReview Request: Bug 1199885 - Part 9: Let APZC handle the drag events. r=kats Bug 1199885 - Part 9: Let APZC handle the drag events. r=kats

Comment on attachment 8660860 [details] MozReview Request: Bug 1199885 - Part 9.5: Make the mouse events APZC aware. r?kats Bug 1199885 - Part 9.5: Make the mouse events APZC aware. r?kats

Comment on attachment 8660861 [details] MozReview Request: Bug 1199885 - Part 10: Add APZTeeManager API to start an async scroll. r?kats Bug 1199885 - Part 10: Add APZTeeManager API to start an async scroll. r?kats

Comment on attachment 8660862 [details] MozReview Request: Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r?kats Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r?kats

Comment on attachment 8660863 [details] MozReview Request: Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r?kats

Bug 1199885 - Part 14: Add input blocks for mouse events. r?kats

https://reviewboard.mozilla.org/r/19169/#review18985 ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1001 (Diff revision 3) > + CSSPoint scrollFramePoint = aEvent.mLocalOrigin / GetFrameMetrics().GetZoom(); This function calls GetFrameMetrics() a lot, would be good to cache it locally, or to just use mFrameMetrics directly. ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1005 (Diff revision 3) > + CSSRect cssCompositionBound = GetFrameMetrics().GetCompositionBounds() / GetFrameMetrics().GetZoom(); You can use GetFrameMetrics().CalculateCompositedRectInCssPixels() ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1031 (Diff revision 3) > + scrollOffset = CSSPoint(scrollPosition, 0); Using 0 here I think means that if you're halfway down a page and then start scrolling horizontally, you'll snap back up to the top. That's not desirable, you should use the existing scroll offset's y coord. ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1039 (Diff revision 3) > + // Here we consume the events. This means that the content scrollbars This comment is wrong; the return value from this function is eventually dropped and has no bearing on whether the content scrollbars see this event or not. ::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1095 (Diff revision 3) > + ScrollWheelInput scrollInput = aEvent.AsScrollWheelInput(); This is wrong, it's converting MOUSE_INPUT types to ScrollWheelInput. Copy/paste error most likely. Since this hunk is removed in part 14 anyway, you can just take it out in this patch instead.

https://reviewboard.mozilla.org/r/19171/#review18987 ::: dom/ipc/TabChild.cpp:1940 (Diff revision 3) > + nsEventStatus unused; You should still initialize this to something sane so it's not garbage.

https://reviewboard.mozilla.org/r/19173/#review18989 ::: gfx/layers/apz/src/APZCTreeManager.cpp:643 (Diff revision 3) > + return aEvent.mMessage == eMouseMove || I'm assuming here that you've exercised this code locally with the pref enabled and that it does work. In which case I'm wondering when does the input mouse event have a message type of eDragStart or eDragEnd? Those types are handled in the WidgetMouseEvent -> MouseInput conversion but I would expect them to be handled here as well.

Comment on attachment 8668668 [details] MozReview Request: Bug 1199885 - Part 14: Add input blocks for mouse events. r?kats https://reviewboard.mozilla.org/r/21011/#review18991 ::: gfx/layers/apz/src/InputBlockState.cpp:243 (Diff revision 1) > + return "mouse wheel"; copy/paste error ::: gfx/layers/apz/src/InputQueue.h:11 (Diff revision 1) > +#include "TouchCounter.h" Not sure why these includes are needed here, doesn't seem like they should be. ::: gfx/layers/apz/src/InputQueue.cpp:179 (Diff revision 1) > + bool newBlock = aEvent.mType == MouseInput::MOUSE_DOWN && aEvent.IsLeftButton(); So the way I understand this code, it's going to create a new block any time the user pushes the left button down, and all start putting mouse events into that block until either the next mouse-down happens, or some other input type comes along and breaks the block. This means things like mouse-move events that are outside a drag can also end up in blocks, and I'm not a fan of this. In comment 16 I described what I would like the input blocks to be like - specific to drag gestures, rather than generic mouse input blocks. Converting the way these blocks work from "mouse blocks" to "drag blocks" will simplify a lot of stuff. You probably won't need things like mIsInDrag, and won't need to un-const functions like DispatchEvent. I looked over the bits that aren't directly related to the input block implementation (i.e. mostly the conversion in the widget/ code) and that looks ok, but I'm holding off on reviewing the bulk of this patch because changing it to drag blocks will be significant. I also thought we decided in subsequent comments (19, 20, 26) to not do the input block stuff now, and I wasn't aware you were going to implement this. Sorry if you told me and I missed it. I'm happy with parts 1-13 landing as-is (modulo the comments I just made above) but this part 14 will need a bunch more work to get landed. If you want to continue cleaning this up I'd suggest forking a separate bug to do this and land parts 1-13 before they rot. What do you think? Please also note that I'll be on PTO October 6-22 so if you want to land it all together this part would need to be made landable before October 6, or it will have to wait until after I'm back. ::: widget/cocoa/nsChildView.mm (Diff revision 1) > - DispatchEvent(&event, status); Why did you remove this line?

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #98) > https://reviewboard.mozilla.org/r/19171/#review18987 > > ::: dom/ipc/TabChild.cpp:1940 > (Diff revision 3) > > + nsEventStatus unused; > > You should still initialize this to something sane so it's not garbage. I actually just had a similar discussion with Jeff today about another problem. In this case we don't really have any value that are meaningful when initialized. By initializing this to an incorrect value it means that tools that warn for 'read without initializing' wont work and thus we might start using a bad value. Leaving this as garbage is better IMO. Perhaps the best fix here is to add an overload of InputAPZContext that doesn't take this out param.

(In reply to Benoit Girard (:BenWa) from comment #101) > I actually just had a similar discussion with Jeff today about another > problem. In this case we don't really have any value that are meaningful > when initialized. By initializing this to an incorrect value it means that > tools that warn for 'read without initializing' wont work and thus we might > start using a bad value. Leaving this as garbage is better IMO. That's a good point. Ok, we can leave it as garbage then.

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #100) > ::: gfx/layers/apz/src/InputQueue.cpp:179 > (Diff revision 1) > > + bool newBlock = aEvent.mType == MouseInput::MOUSE_DOWN && aEvent.IsLeftButton(); > > So the way I understand this code, it's going to create a new block any time > the user pushes the left button down, and all start putting mouse events > into that block until either the next mouse-down happens, or some other > input type comes along and breaks the block. This means things like > mouse-move events that are outside a drag can also end up in blocks, and I'm > not a fan of this. As we discussed on IRC we're going to change this to be called a drag block and only consider events between a mouse down and mouse up block. > > In comment 16 I described what I would like the input blocks to be like - > specific to drag gestures, rather than generic mouse input blocks. > Converting the way these blocks work from "mouse blocks" to "drag blocks" > will simplify a lot of stuff. You probably won't need things like mIsInDrag, > and won't need to un-const functions like DispatchEvent. I looked over the > bits that aren't directly related to the input block implementation (i.e. > mostly the conversion in the widget/ code) and that looks ok, but I'm > holding off on reviewing the bulk of this patch because changing it to drag > blocks will be significant. Yes, I think you're right and it will make this easier. I'll have a shot at it. > > I also thought we decided in subsequent comments (19, 20, 26) to not do the > input block stuff now, and I wasn't aware you were going to implement this. > Sorry if you told me and I missed it. > > I'm happy with parts 1-13 landing as-is (modulo the comments I just made > above) but this part 14 will need a bunch more work to get landed. If you > want to continue cleaning this up I'd suggest forking a separate bug to do > this and land parts 1-13 before they rot. What do you think? Please also > note that I'll be on PTO October 6-22 so if you want to land it all together > this part would need to be made landable before October 6, or it will have > to wait until after I'm back. > My aim was to get this patch set working since as the review progressed, the feature no longer worked without input blocks. I don't want to land a huge patch queue that's effectively dead code so I start to work on the last part now. I'll move it to a separate bug. We're still going to need to a follow up to deal with wheel and mouse/drag blocks mixing. Right now if you click during a wheel scroll the wheel scroll stops abruptly. That will likely be a follow-up. > ::: widget/cocoa/nsChildView.mm > (Diff revision 1) > > - DispatchEvent(&event, status); > > Why did you remove this line? I'm not sure, maybe a vim accident while searching. It's restored now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c267e8cc7952253892b062498d21afd7b10559dd Bug 1199885 - Part 1: Add MouseInput InputData. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/60cbca9b66500267658a551be21c6ae7a0f5b213 Bug 1199885 - Part 2: Add AsyncDragMetrics. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/853289b175764b66f72d5557bb84697934678fdf Bug 1199885 - Part 3: Add 'apz.drag.enabled' preference for async scrollbars. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/012607cf35d59a3bf6b3e0c14d91adec87b1773e Bug 1199885 - Part 4: Let the hit testing tree know about the scroll thumb. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/f650905bf686fa0139c7c96ada68c6590eb34809 Bug 1199885 - Part 5: Make mRootLayerTreeID const to prove that there's no data races. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/c6f50e9286b663bf41da5ba01f9a54f45782c06e Bug 1199885 - Part 8: Add FindScrollNode to locate the scrollbar thumb. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/0c27f0511f8893bc673024d32317dcc76d0e8489 Bug 1199885 - Part 9: Let APZC handle the drag events. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/f69028be4a423c5c56fc7101b33e921a3e4c2c28 Bug 1199885 - Part 9.5: Make the mouse events APZC aware. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/266c7e4df84e81a5f13cb14a1caa389df92b7ee6 Bug 1199885 - Part 10: Add APZTeeManager API to start an async scroll. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/c9999ccc9a8b7f77ad54b3540196c2725a82bb7b Bug 1199885 - Part 12: Add StartScrollbarDrag IPC message. r=kats https://hg.mozilla.org/integration/mozilla-inbound/rev/ccaad97f4f64210abd20219d9466e4f589accb53 Bug 1199885 - Part 13: Let nsSliderFrame trigger async scrolling via StartScrollbarDrag. r=kats

https://hg.mozilla.org/mozilla-central/rev/c267e8cc7952 https://hg.mozilla.org/mozilla-central/rev/60cbca9b6650 https://hg.mozilla.org/mozilla-central/rev/853289b17576 https://hg.mozilla.org/mozilla-central/rev/012607cf35d5 https://hg.mozilla.org/mozilla-central/rev/f650905bf686 https://hg.mozilla.org/mozilla-central/rev/c6f50e9286b6 https://hg.mozilla.org/mozilla-central/rev/0c27f0511f88 https://hg.mozilla.org/mozilla-central/rev/f69028be4a42 https://hg.mozilla.org/mozilla-central/rev/266c7e4df84e https://hg.mozilla.org/mozilla-central/rev/c9999ccc9a8b https://hg.mozilla.org/mozilla-central/rev/ccaad97f4f64