Fix stale platform view gr context on iOS #13469
Conversation
| @@ -0,0 +1,7 @@ | |||
| // This is a generated file; do not edit or check into version control. | |||
There was a problem hiding this comment.
I think you didn't mean to check this in
| @@ -0,0 +1,8 @@ | |||
| #!/bin/sh | |||
| @@ -0,0 +1,14 @@ | |||
| // | |||
| @@ -0,0 +1,12 @@ | |||
| // | |||
amirh
force-pushed
the
fix_stale_gr_context
branch
from
037cc6e
to
f3ddab3
Compare
| @@ -35,6 +35,29 @@ - (instancetype)initWithInvocation:(NSInvocation*)invocation { | |||
| } | |||
|
|
|||
| - (void)testPlatformView { | |||
| //[self checkGolden]; | |||
| [[XCUIDevice sharedDevice] pressButton:XCUIDeviceButtonHome]; | ||
| [self.application activate]; | ||
| [self checkGolden]; | ||
| } |
There was a problem hiding this comment.
It seems we are also testing the background and foreground thingy here. Maybe we should rename the test to be more specific about it? (Like what is the difference between this test and the test below)
Or if you didn't mean to test the background and foreground here, you can remove the logic.
There was a problem hiding this comment.
🤦♂ this part should be reverted, I started by experimenting on this test. I'll followup in a new PR
| } | ||
|
|
||
| - (void)testPlatformView { | ||
| [[XCUIDevice sharedDevice] pressButton:XCUIDeviceButtonHome]; |
There was a problem hiding this comment.
Suggestion:
It seems like we would encounter a false positive here if Flutter didn't draw the rectangle before going to the background.
We might want to add a golden check before going to the background just to be on the safe side?
Maybe add a new method checkGoldenWithName for it?
There was a problem hiding this comment.
Isn't that covered by the other MultiPlatformViews test (which is drawing a circle on top of the platform views)?
|
@cyanglaz I missed your comments before merging, I'll follow up in a separate PR |
When gr context is changed (this happens when sending the app to the background and then to the foreground) we need to update it for all the platform view overlay surfaces.
The update logic was caching the previous gr context to figure if it had to be updated, but after updating it for a given overlay we were updating the cached context. In apps with multiple platform views this will result in overlays with a stale gr context.
This fixes flutter/flutter#36437
And I believe it should fix flutter/flutter#36999 as well (though I don't have repro code to verify).