Closed Bug 1404273 Opened 7 years ago Closed 7 years ago

[Pointer Event] Coalescing mousemove for different pointers

Categories

(Core :: DOM: Events, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: stone, Assigned: stone)

References

Details

Attachments

(1 file, 1 obsolete file)

According to [1], we should coalesce the mousemove / pointermove events which belong to the same pointer (has the same poiniter id)

[1] https://w3c.github.io/pointerevents/extension.html
Assignee: nobody → sshih
Blocks: 1303957
Attachment #8915779 - Flags: review?(bugs)
Comment on attachment 8915779 [details] [diff] [review]
Coalescing mousemove for different pointers

> TabChild::MaybeDispatchCoalescedMouseMoveEvents()
> {
>-  if (!mCoalesceMouseMoveEvents || mCoalescedMouseData.IsEmpty()) {
>+  if (!mCoalesceMouseMoveEvents) {
>     return;
>   }
>-  const WidgetMouseEvent* event = mCoalescedMouseData.GetCoalescedEvent();
>-  MOZ_ASSERT(event);
>-  // Dispatch the coalesced mousemove event. Using RecvRealMouseButtonEvent to
>-  // bypass the coalesce handling in RecvRealMouseMoveEvent.
>-  RecvRealMouseButtonEvent(*event,
>-                           mCoalescedMouseData.GetScrollableLayerGuid(),
>-                           mCoalescedMouseData.GetInputBlockId());
>-  if (mCoalescedMouseEventFlusher) {
>-    mCoalescedMouseData.Reset();
>-    mCoalescedMouseEventFlusher->RemoveObserver();
>+  for (auto iter = mCoalescedMouseData.Iter(); !iter.Done(); iter.Next()) {
>+    CoalescedMouseData* data = iter.UserData();
>+    if (!data || data->IsEmpty()) {
>+      continue;
>+    }
>+    const WidgetMouseEvent* event = data->GetCoalescedEvent();
>+    MOZ_ASSERT(event);
>+    // Dispatch the coalesced mousemove event. Using RecvRealMouseButtonEvent to
>+    // bypass the coalesce handling in RecvRealMouseMoveEvent.
>+    RecvRealMouseButtonEvent(*event,
>+                             data->GetScrollableLayerGuid(),
>+                             data->GetInputBlockId());
>+    data->Reset();
>+    if (mCoalescedMouseEventFlusher) {
>+      mCoalescedMouseEventFlusher->RemoveObserver();
>+    }
Why do we call RemoveObserver inside the for loop?
Move this to be outside the loop?

Since CoalescedMouseData is now explicitly allocated, please add CTOR and DTOR counters to its constructor/destructor.
Attachment #8915779 - Flags: review?(bugs) → review+
Priority: -- → P1
Followed review comments to revise the patch
Attachment #8915779 - Attachment is obsolete: true
Attachment #8916828 - Flags: review+
Pushed by sshih@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e0a1d82ef67
[Pointer Event] Coalescing mousemove for different pointers. r=smaug.
https://hg.mozilla.org/mozilla-central/rev/5e0a1d82ef67
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Depends on: 1407350
You need to log in before you can comment on or make changes to this bug.