Fix PlatformView multiple pointer crash caused by toMotionEvent

[mengdouer]

Fixes flutter/flutter#105229 fixes flutter/flutter#70639

Pre-launch Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
• [x] I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• [x] I listed at least one issue that this PR fixes in the description above.
• [x] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
• [x] I updated/added relevant documentation (doc comments with ///).
• [x] I signed the CLA.
• [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard[bot]]

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 (don't just cc him here, he won't see it! He's on Discord!).

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.

[xanahopper]

This PR will fix the following issues

• #105229
• #70369
• #88696
• #100745
• #61322

But using reconstruct event from Flutter engine may cause PlatformView receive a single-touch event instead of a multi-touch event and missing some touch point infomation.

Please make sure the original behavior of multi-touch event with different view in Android.

[wangying3426]

@iskakaushik Could you have time to look at this PR?

[chinmaygarde]

From Triage: It is hard to reason about the correctness of this patch. Can you write a test or describe the issue and fix in more detail?

[mengdouer]

@chinmaygarde When the gesture event comes, it is stored in the trackedEvent in addPointerForIndex. When the PlatformView wants to respond to the gesture event later, it mixes the MotionEvent synthesized from flutter and the attributes of the event saved at that time. At this time, the correctness of the ActionIndex in the synthesized Event cannot be guaranteed. For example, ActionIndex has always been 2, but pointerCount has changed from 3 to 2, so a crash will occur (in general, it will be caught but the special mechanism of chromium will throw it out)

if (trackedEvent != null) {
      return MotionEvent.obtain(
          trackedEvent.getDownTime(),
          trackedEvent.getEventTime(),
          touch.action,
          touch.pointerCount,
          pointerProperties,
          pointerCoords,
          trackedEvent.getMetaState(),
          trackedEvent.getButtonState(),
          trackedEvent.getXPrecision(),
          trackedEvent.getYPrecision(),
          trackedEvent.getDeviceId(),
          trackedEvent.getEdgeFlags(),
          trackedEvent.getSource(),
          trackedEvent.getFlags());
    }

We are also thinking about other solutions, like @xanahopper mentioned above may affect the behavior of multi-point contact, if you have any ideas, you can share them here

[chinmaygarde]

Thanks for elaborating. cc @dnfield. This probably still needs a test.

[mengdouer]

Thanks for elaborating. cc @dnfield. This probably still needs a test.

Sorry for not replying for so long. Not sure if this also needs to be tested. As another example, when two or more events are sent to the framework side, but some of the current events are processed by the framework side, for example, platformview and a Container are on the same stack, and the first event falls to the Container. It is not transparent, and the rest is sent back to platformview. When toMotionEvent, the actionIndex is still sent before, such as 2, but because some events are intercepted, the check will fail.

[mengdouer]

Below is where the error occurs

// frameworks/base/core/jni/android_view_MotionEvent.cpp
static jint android_view_MotionEvent_nativeGetPointerId(JNIEnv* env, jclass clazz,
        jlong nativePtr, jint pointerIndex) {
    MotionEvent* event = reinterpret_cast<MotionEvent*>(nativePtr);
    size_t pointerCount = event->getPointerCount();
    if (!validatePointerIndex(env, pointerIndex, pointerCount)) {
        return -1;
    }
    return event->getPointerId(pointerIndex);
}

static bool validatePointerIndex(JNIEnv* env, jint pointerIndex, size_t pointerCount) {
    if (pointerIndex < 0 || size_t(pointerIndex) >= pointerCount) {
        jniThrowException(env, "java/lang/IllegalArgumentException",
                "pointerIndex out of range");
        return false;
    }
    return true;
}

I think the action of the event handled by platformView should be consistent with the one sent from the framework side, will this cause other problems?This is a bug fix, but I saw that there is no similar test, what do I need to do? Does anyone have time to take a look at this pr? @chinmaygarde @iskakaushik

[dnfield]

The test for this would be similar to itUsesActionEventTypeFromFrameworkEventForVirtualDisplays in PlatformViewsControllerTest.java. Do you think you can take that on?

[dnfield]

/cc @stuartmorgan FYI

[mengdouer]

@dnfield Thank you for your suggestion. I added the test but the test seems to fail in the compilation engine. I was successful locally, could you help me to find the problem?

[dnfield]

Try merging or rebasing on main - it looks like there may have been changes in CI since you branched that are incompatible.

[mengdouer]

@dnfield All the tests have passed, could you please review again

[dnfield]

@stuartmorgan or @iskakaushik would you mind providing a second review here?

[wangying3426]

This fix works fine in our business, so also LGTM.