Fix crash on multi-finger gesture (#69431) - #78874

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

In which file should the test be added? It is not very trivial to hit the new if condition correctly.

Or rather: Does this PR even need a test? Line 573 in the same file is not tested either on master.

This should definitely have a test. Ideally we should also add a test for any other lines in that file lacking coverage.

@Hixie So, I found two other possible approaches to this problem. I am not sure, if these are the right way to tackle this, but they also get the job done. Please let me know if one of these is better than the current approach.

Approach 1

The fault seems to be, that updatePointerPositions is called when it should not be called, i.e. the corresponding pointer is not registered as being down yet/anymore. Thus, we can just add a simple check around these statements like this:

if (pointerProperties[event.pointer] != null) {
  // or pointerProperties.keys.contains(event.pointer), don't know what's better
  ...
}

There is however one downside to this approach: Since the downTimeMillis get also reset in a cancel event, we need to add a null check for it in the toAndroidMotionEvent method instead of these lines as well:

    // A pointer event seems to have been cancelled and therefore, no motion event
    // can be created.
    if (downTimeMillis == null) {
      return null;
    }

Approach 2

Since the pointerPositions list is always bigger than the pointerProperties list and Android seems to update the pointer positions, which leads to the error, we can just change the pointers list to be initialized like this instead (notice the change from pointerPositions to pointerProperties:

final List<int> pointers = pointerProperties.keys.toList();

However, also in this case the downTimeMillis get reset such that the null check is needed inside the toAndroidMotionEevent function.

Approach 3

See this PR's commit

This is the wrong repo to fix this. It should be fixed in the Android-specific code in the flutter/engine repo. The framework is supposed to be able to rely on a coherent set of messages from the engine.

I tried looking into the engine, but I don't have enough faith in my knowledge with this kind of native stuff. So, sadly my journey ends here and anyone else has to look into this, but thank you! :/

This is the wrong repo to fix this. It should be fixed in the Android-specific code in the flutter/engine repo. The framework is supposed to be able to rely on a coherent set of messages from the engine.

After debugging and looking at the log output, I don't think it's the engine problem, but the system Rom problem.
My test phone is OnePlus 7 and Android 10. When the "three-finger screenshot" function is enabled, ROM will do special treatment for the three-finger event. From the log analysis, ROM intercepts the DownEvent and directly sends the CancelEvent, resulting in a program exception.
Therefore, I think we should judge the length of pointerProperties and pointerPositions in the toAndroidMotionEvent method, so as to be compatible with phones with three-finger screenshot function, or adopt other modification schemes.

The changed code of flutter/src/services/platform_views.dart is as follows：

  @override
  Future<void> dispatchPointerEvent(PointerEvent event) async {
    print('=========${event.runtimeType}'); // new  add
    if (event is PointerDownEvent) {
      _motionEventConverter.handlePointerDownEvent(event);
    }
     ......
   }

 AndroidMotionEvent? toAndroidMotionEvent(PointerEvent event) {
    .....
    if (event.platformData == kPointerDataFlagBatched ||
        (isSinglePointerAction(event) && pointerIdx < numPointers - 1)) {
      print('========= return null pointerProperties.length = ${pointerProperties.length} pointerPositions.length = ${pointerPositions.length}');  // new add
      return null;
    }
    print('========= pointerProperties.length = ${pointerProperties.length} pointerPositions.length = ${pointerPositions.length}');  //new  add
     .....
  }

  void handlePointerUpEvent(PointerUpEvent event) {
    print('=========handlePointerUpEvent'); // new add
    pointerPositions.remove(event.pointer);
    usedAndroidPointerIds.remove(pointerProperties[event.pointer]!.id);
    pointerProperties.remove(event.pointer);
    if (pointerProperties.isEmpty) {
      downTimeMillis = null;
    }
  }

  void handlePointerCancelEvent(PointerCancelEvent event) {
    print('=========handlePointerCancelEvent'); // new add
    pointerPositions.clear();
    pointerProperties.clear();
    usedAndroidPointerIds.clear();
    downTimeMillis = null;
  }

The log output when using the three-finger swipe screen without the "three-finger screenshot" feature enabled.

With the "three-finger screenshot" feature enabled, the log output when using the three-finger swipe screen is.

As you can see, when the "three-finger screenshot" function is enabled, because the system intercepts the DownEvent and sends the CancelEvent incorrectly, the handlePointerCancelEvent is called, pointerPositions and pointerProperties are emptied at the same time, but then the MoveEvent of the second finger is received, the MoveEvent calls the updatePointerPositions and adds an AndroidPointerCoords object to the pointerPositions. At this time, pointerPositions.length is 1, but pointerProperties.length is still 0 at this time.

  void updatePointerPositions(PointerEvent event) {
    final Offset position = _pointTransformer(event.position);
    pointerPositions[event.pointer] = AndroidPointerCoords(
      orientation: event.orientation,
      pressure: event.pressure,
      size: event.size,
      toolMajor: event.radiusMajor,
      toolMinor: event.radiusMinor,
      touchMajor: event.radiusMajor,
      touchMinor: event.radiusMinor,
      x: position.dx,
      y: position.dy,
    );
  }

As far as I can see, this is nothing new as I already debugged this and described the behavior in the issue itself.

Thank you anyways for debugging and veifiying that the underlying issue is the same on your device.

As far as I can see, this is nothing new as I already debugged this and described the behavior in the issue itself.

Thank you anyways for debugging and veifiying that the underlying issue is the same on your device.

The reason for the detailed log instructions is to explain to Ian Hickson that this is not an engine error, but the phone manufacturer changed the system, which caused the three finger operation did not send out the correct event, so flutter should incorporate this modified code to solve the compatibility problem. Of course, other solutions can also be adopted. Anyway, I hope this problem can be solved as soon as possible.