Fix for Android Visibility/Opacity crash #785

Merged
merged 5 commits into from Mar 3, 2017

Conversation

Projects
None yet
6 participants
@hartez
Member

hartez commented Feb 25, 2017

Description of Change

Restoring the visibility of Views which have an Alpha value between 0 and 1 causes a crash due to a negative width/height in VisualElementTracker's UpdateLayout method.

Bugs Fixed

API Changes

None

Behavioral Changes

None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Feb 25, 2017

Contributor

OP indicated that the issue started happening in 2.3.3-pre3. Looking at commit history and stacktrace, it might have something to do with 87a79d1, but one needs to verify this is the case.

01-20 11:23:35.923 E/AndroidRuntime(24322): java.lang.IllegalStateException: Unable to create layer for Platform_DefaultRenderer
	at android.os.MessageQueue.nativePollOnce(Native Method)
	at android.os.MessageQueue.next(MessageQueue.java:323)
	at android.os.Looper.loop(Looper.java:143)
	at android.app.ActivityThread.main(ActivityThread.java:7229)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1230)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1120)
Contributor

adrianknight89 commented Feb 25, 2017

OP indicated that the issue started happening in 2.3.3-pre3. Looking at commit history and stacktrace, it might have something to do with 87a79d1, but one needs to verify this is the case.

01-20 11:23:35.923 E/AndroidRuntime(24322): java.lang.IllegalStateException: Unable to create layer for Platform_DefaultRenderer
	at android.os.MessageQueue.nativePollOnce(Native Method)
	at android.os.MessageQueue.next(MessageQueue.java:323)
	at android.os.Looper.loop(Looper.java:143)
	at android.app.ActivityThread.main(ActivityThread.java:7229)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1230)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1120)
+ PlatformAffected.Android)]
+ public class Bugzilla51238 : TestContentPage
+ {
+#if UITEST

This comment has been minimized.

@samhouts

samhouts Feb 27, 2017

Member

You're freaking me out, having this first. :P

@samhouts

samhouts Feb 27, 2017

Member

You're freaking me out, having this first. :P

@hartez hartez changed the title from Workaround for Android Visibility/Opacity crash to Workaround for Android Visibility/Opacity crash [DO NOT MERGE] Feb 28, 2017

@hartez hartez changed the title from Workaround for Android Visibility/Opacity crash [DO NOT MERGE] to Fix for Android Visibility/Opacity crash Feb 28, 2017

@hartez

This comment has been minimized.

Show comment
Hide comment
@hartez

hartez Feb 28, 2017

Member

@adrianknight89 Thanks for tracking down that change - you're right, the bug did show up then. Turns out that change revealed a problem with negative size values in VisualElementTracker.

Member

hartez commented Feb 28, 2017

@adrianknight89 Thanks for tracking down that change - you're right, the bug did show up then. Turns out that change revealed a problem with negative size values in VisualElementTracker.

@hartez

This comment has been minimized.

Show comment
Hide comment
@hartez

hartez Feb 28, 2017

Member

@samhouts Can I get another review for the new fix? This one has a lot less code :)

Member

hartez commented Feb 28, 2017

@samhouts Can I get another review for the new fix? This one has a lot less code :)

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Feb 28, 2017

Contributor

Great :)

Contributor

adrianknight89 commented Feb 28, 2017

Great :)

@@ -3,6 +3,8 @@
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.ComponentModel;
+using System.Diagnostics.CodeAnalysis;

This comment has been minimized.

@samhouts

samhouts Feb 28, 2017

Member

Don't think we need these.

@samhouts

samhouts Feb 28, 2017

Member

Don't think we need these.

hartez added some commits Feb 24, 2017

@rmarinho rmarinho merged commit caf2e81 into master Mar 3, 2017

6 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passe…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: Windows Debug : Tests passed: 3743, ignored: 10
Details
iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests p…
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests pa…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests pa…
Details

@StephaneDelcroix StephaneDelcroix deleted the fix-bugzilla51238 branch Mar 3, 2017

rmarinho added a commit that referenced this pull request Mar 3, 2017

Fix for Android Visibility/Opacity crash (#785)
* Repro

* Fix for UI test

* Cache the Alpha value and queue up restoration after the visibility is changed

* Fix issue with negative height/width in UpdateLayout

* Clean up usings

jonathanpeppers pushed a commit to Hitcents/Xamarin.Forms that referenced this pull request Mar 30, 2017

Fix for Android Visibility/Opacity crash (#785)
* Repro

* Fix for UI test

* Cache the Alpha value and queue up restoration after the visibility is changed

* Fix issue with negative height/width in UpdateLayout

* Clean up usings

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment