Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reland "[ios_platform_view] only recycle maskView when the view is applying mutators #41573" #42115

Merged
merged 7 commits into from May 22, 2023

Conversation

cyanglaz
Copy link
Contributor

The original PR (#41573) was reverted due to flutter/flutter#126951

The issue will be fixed in #42079

This needs to be landed after #42079

fixes: flutter/flutter#125620

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@cyanglaz cyanglaz marked this pull request as ready for review May 18, 2023 00:48
@chinmaygarde
Copy link
Member

Per the description, this is waiting on #42079

@cyanglaz
Copy link
Contributor Author

Per the description, this is waiting on #42079

We decided to combine two PRs for easier cherry-pick. I will merge #42079 into this PR before landing this.

@cyanglaz
Copy link
Contributor Author

cyanglaz commented May 19, 2023

Merged #42079

@dkwingsmt @hellohuanlin Could you give it a quick review to ensure the test is good?

@cyanglaz cyanglaz requested a review from dkwingsmt May 19, 2023 17:24
@hellohuanlin
Copy link
Contributor

i think i reviewed the original PR. is there any changes since then? otherwise we are good to go.

@cyanglaz
Copy link
Contributor Author

i think i reviewed the original PR. is there any changes since then? otherwise we are good to go.

No change, just merged directly.

@cyanglaz cyanglaz added the autosubmit Merge PR when tree becomes green via auto submit App label May 19, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 19, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented May 19, 2023

auto label is removed for flutter/engine, pr: 42115, due to - The status or check suite Linux mac_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@cyanglaz cyanglaz added the autosubmit Merge PR when tree becomes green via auto submit App label May 19, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 19, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented May 19, 2023

auto label is removed for flutter/engine, pr: 42115, due to - The status or check suite Windows windows_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label May 22, 2023
Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM for the test change

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM for the test change

@auto-submit auto-submit bot merged commit 266524c into flutter:main May 22, 2023
29 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 22, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 23, 2023
…127364)

flutter/engine@a342a91...2a325ee

2023-05-22 skia-flutter-autoroll@skia.org Roll Dart SDK from b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter/engine#42224)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Return image decoder error messages to the Dart API (flutter/engine#42175)
2023-05-22 5236035+fzyzcjy@users.noreply.github.com Again a two-word super tiny typo (flutter/engine#42181)
2023-05-22 ychris@google.com Reland "[ios_platform_view] only recycle maskView when the view is applying mutators #41573" (flutter/engine#42115)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Use untransformed text bounds to calculate the size of ColorSourceTextContents (flutter/engine#42142)
2023-05-22 jonahwilliams@google.com [Impeller] Add UV compute shader. (flutter/engine#42192)
2023-05-22 jonahwilliams@google.com [Impeller] remove final cmd buffer waitUntilScheduled on physical iOS (flutter/engine#42160)

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
zanderso added a commit that referenced this pull request May 23, 2023
zanderso added a commit to flutter/flutter that referenced this pull request May 23, 2023
Reverts Engine rolls for bad commit in
flutter/engine#42115


6549765

194d30a

Engine commit is being reverted in
flutter/engine#42231
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2023
zanderso pushed a commit to flutter/flutter that referenced this pull request May 23, 2023
…sions) (#127369)

Manual roll requested by zra@google.com

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

flutter/engine@a342a91...2a325ee

2023-05-22 skia-flutter-autoroll@skia.org Roll Dart SDK from b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter/engine#42224)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Return image decoder error messages to the Dart API (flutter/engine#42175)
2023-05-22 5236035+fzyzcjy@users.noreply.github.com Again a two-word super tiny typo (flutter/engine#42181)
2023-05-22 ychris@google.com Reland "[ios_platform_view] only recycle maskView when the view is applying mutators flutter#41573" (flutter/engine#42115)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Use untransformed text bounds to calculate the size of ColorSourceTextContents (flutter/engine#42142)
2023-05-22 jonahwilliams@google.com [Impeller] Add UV compute shader. (flutter/engine#42192)
2023-05-22 jonahwilliams@google.com [Impeller] remove final cmd buffer waitUntilScheduled on physical iOS (flutter/engine#42160)

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
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
…sions) (flutter#127369)

Manual roll requested by zra@google.com

flutter/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/engine#42231)
2023-05-23 skia-flutter-autoroll@skia.org Roll Skia from ac87929b3d2e to
6a57876d0e44 (2 revisions) (flutter/engine#42230)
2023-05-23 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from
QAwORJOkyNl4J3x4Y... to DzmjiSg6XC0JUfbKP... (flutter/engine#42227)
2023-05-23 skia-flutter-autoroll@skia.org Manual roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter/engine#42229)
2023-05-23 skia-flutter-autoroll@skia.org Roll Skia from d448fe07ea46 to
ac87929b3d2e (8 revisions) (flutter/engine#42226)
2023-05-23 dnfield@google.com Make FML_LOG safe from static
initialization (flutter/engine#42219)
2023-05-23 uysalere@gmail.com [fuchsia] Bind ChildViewWatcher on
platform thread (flutter/engine#42222)
2023-05-22 skia-flutter-autoroll@skia.org Roll Dart SDK from
b3e1eeda4918 to 1ca8f8368ecc (5 revisions) (flutter/engine#42224)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Return
image decoder error messages to the Dart API (flutter/engine#42175)
2023-05-22 5236035+fzyzcjy@users.noreply.github.com Again a two-word
super tiny typo (flutter/engine#42181)
2023-05-22 ychris@google.com Reland "[ios_platform_view] only recycle
maskView when the view is applying mutators flutter#41573"
(flutter/engine#42115)
2023-05-22 jason-simmons@users.noreply.github.com [Impeller] Use
untransformed text bounds to calculate the size of
ColorSourceTextContents (flutter/engine#42142)
2023-05-22 jonahwilliams@google.com [Impeller] Add UV compute shader.
(flutter/engine#42192)
2023-05-22 jonahwilliams@google.com [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 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
@cyanglaz cyanglaz deleted the maskviewpool_reland branch June 13, 2023 17:06
auto-submit bot pushed a commit that referenced this pull request Jun 13, 2023
…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
…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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-ios
Projects
None yet
5 participants