Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

[BrowserGestureDetector] Crash in [@ java.lang.IllegalArgumentException: at kotlin.jvm.internal.ArrayIteratorKt.throwParameterIsNullException(ArrayIterator.kt:5)] #8356

Closed
kbrosnan opened this issue Sep 8, 2020 · 3 comments
Assignees
Labels
💥 crash <toolbar> Components: browser-toolbar, concept-toolbar
Milestone

Comments

@kbrosnan
Copy link
Contributor

kbrosnan commented Sep 8, 2020

Crash report: https://crash-stats.mozilla.org/report/index/3e85c676-c77c-4ca8-ab56-1504d0200908

Java stack trace:

java.lang.IllegalArgumentException
	at kotlin.jvm.internal.ArrayIteratorKt.throwParameterIsNullException(ArrayIterator.kt:5)
	at kotlin.jvm.internal.ArrayIteratorKt.checkParameterIsNotNull(ArrayIterator.kt:1)
	at mozilla.components.browser.toolbar.behavior.BrowserGestureDetector$CustomScrollDetectorListener.onScroll(Unknown Source:2)
	at android.view.GestureDetector.onTouchEvent(GestureDetector.java:625)
	at mozilla.components.browser.toolbar.behavior.BrowserGestureDetector.handleTouchEvent$browser_toolbar_release(BrowserGestureDetector.kt:7)
	at mozilla.components.browser.toolbar.behavior.BrowserToolbarBottomBehavior.onInterceptTouchEvent(BrowserToolbarBottomBehavior.kt:2)
	at mozilla.components.browser.toolbar.behavior.BrowserToolbarBottomBehavior.onInterceptTouchEvent(BrowserToolbarBottomBehavior.kt:1)
	at androidx.coordinatorlayout.widget.CoordinatorLayout.performIntercept(CoordinatorLayout.java:13)
	at androidx.coordinatorlayout.widget.CoordinatorLayout.onInterceptTouchEvent(CoordinatorLayout.java:3)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2573)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3030)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2719)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3030)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2719)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3030)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2719)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3030)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2719)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3030)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2719)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3030)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2719)
	at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3030)
	at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2719)
	at com.android.internal.policy.DecorView.superDispatchTouchEvent(DecorView.java:452)
	at com.android.internal.policy.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1836)
	at android.app.Activity.dispatchTouchEvent(Activity.java:3400)
	at androidx.appcompat.view.WindowCallbackWrapper.dispatchTouchEvent(WindowCallbackWrapper.java:1)
	at com.android.internal.policy.DecorView.dispatchTouchEvent(DecorView.java:410)
	at android.view.View.dispatchPointerEvent(View.java:12768)
	at android.view.ViewRootImpl$ViewPostImeInputStage.processPointerEvent(ViewRootImpl.java:5127)
	at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:4930)
	at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4447)
	at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4500)
	at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4466)
	at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:4606)
	at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4474)
	at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:4663)
	at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4447)
	at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4500)
	at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4466)
	at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4474)
	at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4447)
	at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:7124)
	at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:7093)
	at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:7054)
	at android.view.ViewRootImpl$WindowInputEventReceiver.onInputEvent(ViewRootImpl.java:7227)
	at android.view.InputEventReceiver.dispatchInputEvent(InputEventReceiver.java:187)
	at android.view.InputEventReceiver.nativeConsumeBatchedInputEvents(Native Method)
	at android.view.InputEventReceiver.consumeBatchedInputEvents(InputEventReceiver.java:178)
	at android.view.ViewRootImpl.doConsumeBatchedInput(ViewRootImpl.java:7198)
	at android.view.ViewRootImpl$ConsumeBatchedInputRunnable.run(ViewRootImpl.java:7250)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1004)
	at android.view.Choreographer.doCallbacks(Choreographer.java:816)
	at android.view.Choreographer.doFrame(Choreographer.java:745)
	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:990)
	at android.os.Handler.handleCallback(Handler.java:873)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loop(Looper.java:280)
	at android.app.ActivityThread.main(ActivityThread.java:6706)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

┆Issue is synchronized with this Jira Task

@kbrosnan kbrosnan added <toolbar> Components: browser-toolbar, concept-toolbar 💥 crash labels Sep 8, 2020
@pocmo
Copy link
Contributor

pocmo commented Sep 8, 2020

CC @Mugurell

@pocmo pocmo changed the title Crash in [@ java.lang.IllegalArgumentException: at kotlin.jvm.internal.ArrayIteratorKt.throwParameterIsNullException(ArrayIterator.kt:5)] [BrowserGestureDetector] Crash in [@ java.lang.IllegalArgumentException: at kotlin.jvm.internal.ArrayIteratorKt.throwParameterIsNullException(ArrayIterator.kt:5)] Sep 8, 2020
@Mugurell
Copy link
Contributor

Mugurell commented Sep 9, 2020

Thank you Sebastian!

Based on

java.lang.IllegalArgumentException: Parameter specified as non-null is null: method kotlin.jvm.internal.ArrayIteratorKt.checkParameterIsNotNull, parameter e1
at kotlin.jvm.internal.ArrayIteratorKt.throwParameterIsNullException(ArrayIterator.kt:5)
at kotlin.jvm.internal.ArrayIteratorKt.checkParameterIsNotNull(ArrayIterator.kt:1)
at mozilla.components.browser.toolbar.behavior.BrowserGestureDetector$CustomScrollDetectorListener.onScroll(Unknown Source:2)
at android.view.GestureDetector.onTouchEvent(GestureDetector.java:639)

I understand that Android's GestureDetector calls us as a listener with null parameters although it's method definition says the elements are non-nullable. And that first parameter should've been the ACTION_DOWN event.
In testing this I could not reproduce but I think we're seeing cases in which some MotionEvents are not forwarded to the GestureDetector. Otherwise it might be a bug in the GestureDetector.

As a speculative fix I'll

  • refactor out the by lazy(LazyThreadSafetyMode.NONE) GestureDetector initialization. Though all calls should've been on a single Thread and so ensure serial execution.. just to make sure we don't end up with more detectors and somehow some MotionEvents go to one detector and others to another.
  • ensure we always pass ACTION_DOWN, ACTION_UP, ACTION_CANCEL to the detector.

Mugurell added a commit to Mugurell/android-components that referenced this issue Sep 9, 2020
Try to resolve a situation in which system's GestureDetector calls us as a
callback with null parameters, so triggering an IllegalArgumentException.
A possible explanation could be that not all MotionEvents are passed to the
GestureDetector, situation which this patch tries to fix.
Mugurell added a commit to Mugurell/android-components that referenced this issue Sep 9, 2020
Try to resolve a situation in which system's GestureDetector calls us as a
callback with null parameters, so triggering an IllegalArgumentException.
A possible explanation could be that not all MotionEvents are passed to the
GestureDetector, situation which this patch tries to fix.
bors bot pushed a commit that referenced this issue Sep 11, 2020
8357: For #8356 - Speculative fix for BrowserGestureDetector IAE r=pocmo a=Mugurell

Try to resolve a situation in which system's GestureDetector calls us as a
callback with null parameters, so triggering an IllegalArgumentException.
A possible explanation could be that not all MotionEvents are passed to the
GestureDetector, situation which this patch tries to fix.




Co-authored-by: Mugurell <Mugurell@users.noreply.github.com>
@pocmo pocmo added this to the 59.0.0 milestone Sep 15, 2020
@jonalmeida jonalmeida modified the milestones: 59.0.0 🐡, 60.0.0 👽 Sep 17, 2020
Mugurell added a commit to Mugurell/android-components that referenced this issue Sep 30, 2020
…ctor

Last ditch effort to prevent against crashes caused by ACTION_DOWN event being
null in GestureDetector.SimpleOnGestureListener#onScroll.

We can use the fact that the parameters are platform types and so can consider
the first parameter (that should be the ACTION_DOWN MotionEvent) being null so
that we won't automatically crash in such situations.
This will result in the toolbar not being animated as a result of a scroll
gesture by the user.

More investigations about why the ACTION_DOWN event is null are to be done in
android-components/issues/8552.
@Mugurell Mugurell self-assigned this Sep 30, 2020
Mugurell added a commit to Mugurell/android-components that referenced this issue Oct 1, 2020
…ctor

Last ditch effort to prevent against crashes caused by ACTION_DOWN event being
null in GestureDetector.SimpleOnGestureListener#onScroll.

We can use the fact that the parameters are platform types and so can consider
the first parameter (that should be the ACTION_DOWN MotionEvent) being null so
that we won't automatically crash in such situations.
This will result in the toolbar not being animated as a result of a scroll
gesture by the user.

More investigations about why the ACTION_DOWN event is null are to be done in
android-components/issues/8552.
mergify bot pushed a commit that referenced this issue Oct 2, 2020
Last ditch effort to prevent against crashes caused by ACTION_DOWN event being
null in GestureDetector.SimpleOnGestureListener#onScroll.

We can use the fact that the parameters are platform types and so can consider
the first parameter (that should be the ACTION_DOWN MotionEvent) being null so
that we won't automatically crash in such situations.
This will result in the toolbar not being animated as a result of a scroll
gesture by the user.

More investigations about why the ACTION_DOWN event is null are to be done in
android-components/issues/8552.
@Amejia481 Amejia481 modified the milestones: 60.0.0 👽, 62.0.0 🥮 Oct 6, 2020
@jonalmeida
Copy link
Contributor

Closing since this the PR for this was merged in 62.0.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💥 crash <toolbar> Components: browser-toolbar, concept-toolbar
Projects
None yet
Development

No branches or pull requests

5 participants