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
zanderso merged 1 commit into
mainfrom
revert-42115-maskviewpool_reland
May 23, 2023
Merged

Revert "[ios_platform_view] only recycle maskView when the view is applying mutators #41573"#42231
zanderso merged 1 commit into
mainfrom
revert-42115-maskviewpool_reland

Conversation

@zanderso

@zanderso zanderso commented May 23, 2023

Copy link
Copy Markdown
Contributor

@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

@jonahwilliams jonahwilliams left a comment

Copy link
Copy Markdown
Contributor

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

zanderso commented May 23, 2023

Copy link
Copy Markdown
Contributor 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

hellohuanlin commented May 23, 2023

Copy link
Copy Markdown
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 a342a91 (#127368)
bc14fed 
Reverts Engine rolls for bad commit in
flutter-team-archive/engine#42115


6549765

194d30a

Engine commit is being reverted in
flutter-team-archive/engine#42231
@zanderso

zanderso commented May 23, 2023

Copy link
Copy Markdown
Contributor 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
2586cbe Revert "[ios_platform_view] only recycle maskView when the vi…
8a68cbc 
…ew is applying mutators flutter#41573" (flutter-team-archive/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 a342a91 to 2586cbe (14 revisions) (#1…
dd07c61 
…27369)

Manual roll requested by zra@google.com

flutter-team-archive/engine@a342a91...2586cbe

2023-05-23 zanderso@users.noreply.github.com Revert "[ios_platform_view]
only recycle maskView when the view is applying mutators #41573"
(flutter-team-archive/engine#42231)
2023-05-23 skia-flutter-autoroll@skia.org Roll Skia from ac87929b3d2e to
6a57876d0e44 (2 revisions) (flutter-team-archive/engine#42230)
2023-05-23 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
QAwORJOkyNl4J3x4Y... to DzmjiSg6XC0JUfbKP... (flutter-team-archive/engine#42227)
2023-05-23 skia-flutter-autoroll@skia.org Manual roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter-team-archive/engine#42229)
2023-05-23 skia-flutter-autoroll@skia.org Roll Skia from d448fe07ea46 to
ac87929b3d2e (8 revisions) (flutter-team-archive/engine#42226)
2023-05-23 dnfield@google.com Make FML_LOG safe from static
initialization (flutter-team-archive/engine#42219)
2023-05-23 uysalere@gmail.com [fuchsia] Bind ChildViewWatcher on
platform thread (flutter-team-archive/engine#42222)
2023-05-22 skia-flutter-autoroll@skia.org Roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter-team-archive/engine#42224)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Return
image decoder error messages to the Dart API (flutter-team-archive/engine#42175)
2023-05-22 5236035+fzyzcjy@users.noreply.github.com Again a two-word
super tiny typo (flutter-team-archive/engine#42181)
2023-05-22 ychris@google.com Reland "[ios_platform_view] only recycle
maskView when the view is applying mutators #41573"
(flutter-team-archive/engine#42115)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Use
untransformed text bounds to calculate the size of
ColorSourceTextContents (flutter-team-archive/engine#42142)
2023-05-22 jonahwilliams@google.com [Impeller] Add UV compute shader.
(flutter-team-archive/engine#42192)
2023-05-22 jonahwilliams@google.com [Impeller] remove final cmd buffer
waitUntilScheduled on physical iOS (flutter-team-archive/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 rmistry@google.com,zra@google.com 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 a342a91 (flutter#127368)
37ba0c0 
Reverts Engine rolls for bad commit in
flutter-team-archive/engine#42115


flutter@6549765

flutter@194d30a

Engine commit is being reverted in
flutter-team-archive/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 a342a91 to 2586cbe (14 revisions) (fl…
22e1b6d 
…utter#127369)

Manual roll requested by zra@google.com

flutter-team-archive/engine@a342a91...2586cbe

2023-05-23 zanderso@users.noreply.github.com Revert "[ios_platform_view]
only recycle maskView when the view is applying mutators flutter#41573"
(flutter-team-archive/engine#42231)
2023-05-23 skia-flutter-autoroll@skia.org Roll Skia from ac87929b3d2e to
6a57876d0e44 (2 revisions) (flutter-team-archive/engine#42230)
2023-05-23 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
QAwORJOkyNl4J3x4Y... to DzmjiSg6XC0JUfbKP... (flutter-team-archive/engine#42227)
2023-05-23 skia-flutter-autoroll@skia.org Manual roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter-team-archive/engine#42229)
2023-05-23 skia-flutter-autoroll@skia.org Roll Skia from d448fe07ea46 to
ac87929b3d2e (8 revisions) (flutter-team-archive/engine#42226)
2023-05-23 dnfield@google.com Make FML_LOG safe from static
initialization (flutter-team-archive/engine#42219)
2023-05-23 uysalere@gmail.com [fuchsia] Bind ChildViewWatcher on
platform thread (flutter-team-archive/engine#42222)
2023-05-22 skia-flutter-autoroll@skia.org Roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter-team-archive/engine#42224)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Return
image decoder error messages to the Dart API (flutter-team-archive/engine#42175)
2023-05-22 5236035+fzyzcjy@users.noreply.github.com Again a two-word
super tiny typo (flutter-team-archive/engine#42181)
2023-05-22 ychris@google.com Reland "[ios_platform_view] only recycle
maskView when the view is applying mutators flutter#41573"
(flutter-team-archive/engine#42115)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Use
untransformed text bounds to calculate the size of
ColorSourceTextContents (flutter-team-archive/engine#42142)
2023-05-22 jonahwilliams@google.com [Impeller] Add UV compute shader.
(flutter-team-archive/engine#42192)
2023-05-22 jonahwilliams@google.com [Impeller] remove final cmd buffer
waitUntilScheduled on physical iOS (flutter-team-archive/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 rmistry@google.com,zra@google.com 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-team-archive#41573" (flutter-team-archive#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-team-archive#42115" (flutter-team-archive#42823)

Relands flutter-team-archive#42115, which was reverted in flutter-team-archive#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-team-archive#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

@cyanglaz cyanglaz Awaiting requested review from cyanglaz
@hellohuanlin hellohuanlin Awaiting requested review from hellohuanlin
1 more reviewer
@jonahwilliams jonahwilliams jonahwilliams approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

platform-ios

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zanderso @hellohuanlin @jonahwilliams