Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Revert "[ios_platform_view] only recycle maskView when the view is applying mutators #41573" #42231

Merged
merged 1 commit into from
May 23, 2023
Merged

Revert "[ios_platform_view] only recycle maskView when the view is applying mutators #41573" #42231

merged 1 commit into from
May 23, 2023

Conversation

zanderso
Copy link
Member

@zanderso zanderso requested review from cyanglaz and hellohuanlin May 23, 2023 02:29
@zanderso zanderso requested a review from jonahwilliams May 23, 2023 02:29
jonahwilliams
jonahwilliams approved these changes May 23, 2023
View reviewed changes
Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zanderso zanderso mentioned this pull request May 23, 2023
Merged
@zanderso zanderso changed the title Revert "Reland "[ios_platform_view] only recycle maskView when the view is applying mutators #41573"" Revert "[ios_platform_view] only recycle maskView when the view is applying mutators #41573" May 23, 2023
@zanderso
Copy link
Member Author

Eagerly landing revert to get rolls to the framework restarted.

@zanderso zanderso merged commit 2586cbe into main May 23, 2023
@zanderso zanderso deleted the revert-42115-maskviewpool_reland branch May 23, 2023 02:35
@hellohuanlin
Copy link
Contributor

thanks for the revert. curious how did you identify the root cause? did you repro on local machine?

zanderso added a commit to flutter/flutter that referenced this pull request May 23, 2023
@zanderso
Revert Engine to a342a9186e69 (#127368)
bc14fed 
Reverts Engine rolls for bad commit in
flutter/engine#42115


6549765

194d30a

Engine commit is being reverted in
flutter/engine#42231
@zanderso
Copy link
Member Author

In the logs linked above, there is a native crash due to what looks like an error in managing the mask view lifetime:

[2023-05-22 19:08:51.304448] [STDOUT] stdout: [  +67 ms] * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x20)
[2023-05-22 19:08:51.304508] [STDOUT] stdout: [        ]   * frame #0: 0x00000001835aaee4 libobjc.A.dylib`objc_retain_x2 + 16
[2023-05-22 19:08:51.304574] [STDOUT] stdout: [        ]     frame #1: 0x000000018c5fa6d0 UIKitCore`-[UIView _setMaskView:] + 36
[2023-05-22 19:08:51.304625] [STDOUT] stdout: [        ]     frame #2: 0x0000000104394ab0 Flutter`flutter::IOSExternalViewEmbedder::CompositeEmbeddedView(long long) + 1096
[2023-05-22 19:08:51.304673] [STDOUT] stdout: [        ]     frame #3: 0x00000001047dbbd0 Flutter`flutter::PlatformViewLayer::Paint(flutter::PaintContext&) const + 48
[2023-05-22 19:08:51.304768] [STDOUT] stdout: [        ]     frame #4: 0x00000001047d274c Flutter`flutter::ContainerLayer::PaintChildren(flutter::PaintContext&) const + 88
[2023-05-22 19:08:51.304868] [STDOUT] stdout: [        ]     frame #5: 0x00000001047dcd90 Flutter`flutter::TransformLayer::Paint(flutter::PaintContext&) const + 84
...

@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 23, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2023
@engine-flutter-autoroll
2586cbeea Revert "[ios_platform_view] only recycle maskView when the …
8a68cbc 
…view is applying mutators flutter#41573" (flutter/engine#42231)
zanderso pushed a commit to flutter/flutter that referenced this pull request May 23, 2023
@engine-flutter-autoroll
Manual roll Flutter Engine from a342a9186e69 to 2586cbeeae37 (14 revi…
dd07c61 
…sions) (#127369)

Manual roll requested by [email protected]

flutter/engine@a342a91...2586cbe

2023-05-23 [email protected] Revert "[ios_platform_view]
only recycle maskView when the view is applying mutators #41573"
(flutter/engine#42231)
2023-05-23 [email protected] Roll Skia from ac87929b3d2e to
6a57876d0e44 (2 revisions) (flutter/engine#42230)
2023-05-23 [email protected] Roll Fuchsia Mac SDK from
QAwORJOkyNl4J3x4Y... to DzmjiSg6XC0JUfbKP... (flutter/engine#42227)
2023-05-23 [email protected] Manual roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter/engine#42229)
2023-05-23 [email protected] Roll Skia from d448fe07ea46 to
ac87929b3d2e (8 revisions) (flutter/engine#42226)
2023-05-23 [email protected] Make FML_LOG safe from static
initialization (flutter/engine#42219)
2023-05-23 [email protected] [fuchsia] Bind ChildViewWatcher on
platform thread (flutter/engine#42222)
2023-05-22 [email protected] Roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter/engine#42224)
2023-05-22 [email protected] [Impeller] Return
image decoder error messages to the Dart API (flutter/engine#42175)
2023-05-22 [email protected] Again a two-word
super tiny typo (flutter/engine#42181)
2023-05-22 [email protected] Reland "[ios_platform_view] only recycle
maskView when the view is applying mutators #41573"
(flutter/engine#42115)
2023-05-22 [email protected] [Impeller] Use
untransformed text bounds to calculate the size of
ColorSourceTextContents (flutter/engine#42142)
2023-05-22 [email protected] [Impeller] Add UV compute shader.
(flutter/engine#42192)
2023-05-22 [email protected] [Impeller] remove final cmd buffer
waitUntilScheduled on physical iOS (flutter/engine#42160)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from QAwORJOkyNl4 to DzmjiSg6XC0J

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that
a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
@zanderso
Revert Engine to a342a9186e69 (flutter#127368)
37ba0c0 
Reverts Engine rolls for bad commit in
flutter/engine#42115


flutter@6549765

flutter@194d30a

Engine commit is being reverted in
flutter/engine#42231
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
@engine-flutter-autoroll
Manual roll Flutter Engine from a342a9186e69 to 2586cbeeae37 (14 revi…
22e1b6d 
…sions) (flutter#127369)

Manual roll requested by [email protected]

flutter/engine@a342a91...2586cbe

2023-05-23 [email protected] Revert "[ios_platform_view]
only recycle maskView when the view is applying mutators flutter#41573"
(flutter/engine#42231)
2023-05-23 [email protected] Roll Skia from ac87929b3d2e to
6a57876d0e44 (2 revisions) (flutter/engine#42230)
2023-05-23 [email protected] Roll Fuchsia Mac SDK from
QAwORJOkyNl4J3x4Y... to DzmjiSg6XC0JUfbKP... (flutter/engine#42227)
2023-05-23 [email protected] Manual roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter/engine#42229)
2023-05-23 [email protected] Roll Skia from d448fe07ea46 to
ac87929b3d2e (8 revisions) (flutter/engine#42226)
2023-05-23 [email protected] Make FML_LOG safe from static
initialization (flutter/engine#42219)
2023-05-23 [email protected] [fuchsia] Bind ChildViewWatcher on
platform thread (flutter/engine#42222)
2023-05-22 [email protected] Roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter/engine#42224)
2023-05-22 [email protected] [Impeller] Return
image decoder error messages to the Dart API (flutter/engine#42175)
2023-05-22 [email protected] Again a two-word
super tiny typo (flutter/engine#42181)
2023-05-22 [email protected] Reland "[ios_platform_view] only recycle
maskView when the view is applying mutators flutter#41573"
(flutter/engine#42115)
2023-05-22 [email protected] [Impeller] Use
untransformed text bounds to calculate the size of
ColorSourceTextContents (flutter/engine#42142)
2023-05-22 [email protected] [Impeller] Add UV compute shader.
(flutter/engine#42192)
2023-05-22 [email protected] [Impeller] remove final cmd buffer
waitUntilScheduled on physical iOS (flutter/engine#42160)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from QAwORJOkyNl4 to DzmjiSg6XC0J

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that
a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This was referenced Jun 8, 2023
Closed
Closed
cyanglaz pushed a commit to cyanglaz/engine that referenced this pull request Jun 13, 2023
Revert "Revert "[ios_platform_view] only recycle maskView when the vi…
b4736ea 
…ew is applying mutators flutter#41573" (flutter#42231)"

This reverts commit 2586cbe.
@cyanglaz cyanglaz mentioned this pull request Jun 13, 2023
Merged
8 tasks
auto-submit bot pushed a commit that referenced this pull request Jun 13, 2023
Reland "[ios_platform_view] only recycle maskView when the view is ap…
727b441 
…plying mutators #42115" (#42823)

Relands #42115, which was reverted in #42231 due to a crash in the framework test.

The crash is due to a memory management issue that I fixed it in this PR, I also added a scenario test to catch the crash.

fixes: flutter/flutter#125620

See also: orignal PR #41573 for more details.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
cyanglaz pushed a commit to cyanglaz/engine that referenced this pull request Jun 20, 2023
Reland "[ios_platform_view] only recycle maskView when the view is ap…
8970fd1 
…plying mutators flutter#42115" (flutter#42823)

Relands flutter#42115, which was reverted in flutter#42231 due to a crash in the framework test.

The crash is due to a memory management issue that I fixed it in this PR, I also added a scenario test to catch the crash.

fixes: flutter/flutter#125620

See also: orignal PR flutter#41573 for more details.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jonahwilliams jonahwilliams jonahwilliams approved these changes

@cyanglaz cyanglaz Awaiting requested review from cyanglaz

@hellohuanlin hellohuanlin Awaiting requested review from hellohuanlin

Assignees
No one assigned
Labels
platform-ios
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zanderso @hellohuanlin @jonahwilliams