This repository was archived by the owner on Feb 25, 2025. It is now read-only.
  
  
  - 
                Notifications
    
You must be signed in to change notification settings  - Fork 6k
 
[web] Enforce onDrawFrame/onBeginFrame render rule #49214
          
     Merged
      
      
            auto-submit
  merged 6 commits into
  flutter:main
from
harryterkelsen:web-enforce-render-rule
  
      
      
   
  Dec 19, 2023 
      
    
                
     Merged
            
            [web] Enforce onDrawFrame/onBeginFrame render rule #49214
                    auto-submit
  merged 6 commits into
  flutter:main
from
harryterkelsen:web-enforce-render-rule
  
      
      
   
  Dec 19, 2023 
              
            Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    Also update tests to conform to the render rule.
              
                    yjbanov
  
              
              reviewed
              
                  
                    Dec 19, 2023 
                  
              
              
            
            
| void scheduleFrame(); | ||
| 
               | 
          ||
| void render(Scene scene, [FlutterView view]); | ||
| Future<void> render(Scene scene, [FlutterView view]); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the mobile version no longer has this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agh shoot, forgot to remove it from our interface 🤦
              
                    yjbanov
  
              
              approved these changes
              
                  
                    Dec 19, 2023 
                  
              
              
            
            
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
    
  engine-flutter-autoroll 
      added a commit
        to engine-flutter-autoroll/flutter
      that referenced
      this pull request
    
      Dec 20, 2023 
    
    
  
      
        
      
      
  
    8 tasks
  
    
  harryterkelsen 
      added a commit
        to flutter/flutter
      that referenced
      this pull request
    
      Dec 20, 2023 
    
    
      
  
    
      
    
  
The test was flaky before due to overflowing GPU memory during the test. The memory leak was fixed here flutter/engine#49214 Fixes #137669 As a side effect of the fix, this test also runs much faster, from about 3 minutes on my Macbook down to about 25 seconds. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
      
        
      
      
  
    8 tasks
  
    
  auto-submit bot
      added a commit
        to flutter/flutter
      that referenced
      this pull request
    
      Dec 21, 2023 
    
    
      
  
    
      
    
  
…140515) Reverts #140462 Initiated by: cbracken This change reverts the following previous change: Original Description: The test was flaky before due to overflowing GPU memory during the test. The memory leak was fixed here flutter/engine#49214 Fixes #137669 As a side effect of the fix, this test also runs much faster, from about 3 minutes on my Macbook down to about 25 seconds.
| 
           Time to revert pull request flutter/engine/49214 has elapsed.  | 
    
    
  cbracken 
      added a commit
      that referenced
      this pull request
    
      Dec 21, 2023 
    
    
      
  
    
      
    
  
This reverts #49214 and #48758. #49214 appears to cause a Flutter Gold diff during the engine roll. See flutter/flutter#140434 for an example. It's unclear to me whether this is an expected diff and should be approved or unexpected and should be reverting, so reverting to the last previous rollable state. ## 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 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]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
      
        
      
      
  
    8 tasks
  
    
  engine-flutter-autoroll 
      added a commit
        to engine-flutter-autoroll/flutter
      that referenced
      this pull request
    
      Dec 21, 2023 
    
    
  
    
  harryterkelsen 
      added a commit
      that referenced
      this pull request
    
      Dec 21, 2023 
    
    
      
  
    
      
    
  
Enforces the render rule on the web except in the HTML renderer, which must still render to the DOM even outside of `onDrawFrame` or `onBegineFrame` scopes in order for golden tests to continue to work in the framework. This is a reland of #49214 with one change (checking for `renderer.rendererTag == 'html'` in the `render` method). ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
    
  auto-submit bot
      pushed a commit
        to flutter/flutter
      that referenced
      this pull request
    
      Dec 21, 2023 
    
    
      
  
    
      
    
  
…140519) Roll Flutter Engine from c70f0a495ace to 1b1b2a12a597 (32 revisions) flutter/engine@c70f0a4...1b1b2a1 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 4a10533a4dc8 to bcf68d22f0fa (1 revision) (flutter/engine#49329) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 1d0c3ecd1349 to 4a10533a4dc8 (1 revision) (flutter/engine#49326) 2023-12-21 flar@google.com Ensure sorted rects in ui.Canvas for legacy compatibility (flutter/engine#49309) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 63a452b45026 to 1d0c3ecd1349 (1 revision) (flutter/engine#49318) 2023-12-21 dnfield@google.com [Impeller] Make IPLR files multi-platform (flutter/engine#49253) 2023-12-21 ditman@gmail.com [web] Defer injection of platform views until needed. (flutter/engine#48960) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 1aef027ec953 to 63a452b45026 (2 revisions) (flutter/engine#49311) 2023-12-21 skia-flutter-autoroll@skia.org Roll Skia from 29917d8c97ca to 4b16117e94b2 (4 revisions) (flutter/engine#49310) 2023-12-21 737941+loic-sharma@users.noreply.github.com Reland "[Windows] Move to FlutterCompositor for rendering" (flutter/engine#49262) 2023-12-21 30870216+gaaclarke@users.noreply.github.com Reland `[Impeller] new blur: refactored math and fixed expanded padding size` (flutter/engine#49302) 2023-12-20 dkwingsmt@users.noreply.github.com Multiview pipeline Pt. 1: Skip illegal render calls (flutter/engine#49266) 2023-12-20 barpac02@gmail.com SemanticsUpdateBuilder: make all args non-null (flutter/engine#49148) 2023-12-20 30870216+gaaclarke@users.noreply.github.com [Impeller] fixed Rect::Contains (flutter/engine#49294) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 97c3b7e1885a to 1aef027ec953 (1 revision) (flutter/engine#49295) 2023-12-20 30870216+gaaclarke@users.noreply.github.com Revert "[Impeller] new blur: refactored math and fixed expanded padding size" (flutter/engine#49298) 2023-12-20 30870216+gaaclarke@users.noreply.github.com [Impeller] new blur: refactored math and fixed expanded padding size (flutter/engine#49206) 2023-12-20 dkwingsmt@users.noreply.github.com Multi-view pointer event (flutter/engine#46213) 2023-12-20 1961493+harryterkelsen@users.noreply.github.com [web:multiview] Only call `Renderer.clearFragmentProgramCache` on hot restart (flutter/engine#48758) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from 9cb1bb1164ea to 29917d8c97ca (1 revision) (flutter/engine#49289) 2023-12-20 bdero@google.com [Impeller] Add interactive Blur+Clip AiksTest. (flutter/engine#49283) 2023-12-20 sergiy.dubovik@supercell.com [macos] FlutterKeyboardManager memory leak fix (flutter/engine#48824) 2023-12-20 zanderso@users.noreply.github.com Don't guard Windows arm64 Dart SDK download on the release candidate flag (flutter/engine#49244) 2023-12-20 15619084+vashworth@users.noreply.github.com Fix testAppExtensionLaunching for Xcode 15/iOS 17 (flutter/engine#49242) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from 8060d6b36066 to 9cb1bb1164ea (2 revisions) (flutter/engine#49288) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from ed415d966d8a to 97c3b7e1885a (1 revision) (flutter/engine#49287) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from d0f09ad481f7 to 8060d6b36066 (1 revision) (flutter/engine#49285) 2023-12-20 kevinjchisholm@gmail.com [release] Update release config (flutter/engine#49254) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 1732c4c92ccd to ed415d966d8a (1 revision) (flutter/engine#49274) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 4c59838945d9 to 1732c4c92ccd (1 revision) (flutter/engine#49269) 2023-12-20 goderbauer@google.com Sync lints with flutter/flutter (flutter/engine#49192) 2023-12-19 1961493+harryterkelsen@users.noreply.github.com [web] Enforce onDrawFrame/onBeginFrame render rule (flutter/engine#49214) 2023-12-19 barpac02@gmail.com [Docs] Add more info about running tests on iOS (flutter/engine#48859) 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 jimgraham@google.com,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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...
    
  CoderDake 
      pushed a commit
        to CoderDake/flutter
      that referenced
      this pull request
    
      Dec 28, 2023 
    
    
      
  
    
      
    
  
…40462) The test was flaky before due to overflowing GPU memory during the test. The memory leak was fixed here flutter/engine#49214 Fixes flutter#137669 As a side effect of the fix, this test also runs much faster, from about 3 minutes on my Macbook down to about 25 seconds. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
    
  CoderDake 
      pushed a commit
        to CoderDake/flutter
      that referenced
      this pull request
    
      Dec 28, 2023 
    
    
      
  
    
      
    
  
…lutter#140515) Reverts flutter#140462 Initiated by: cbracken This change reverts the following previous change: Original Description: The test was flaky before due to overflowing GPU memory during the test. The memory leak was fixed here flutter/engine#49214 Fixes flutter#137669 As a side effect of the fix, this test also runs much faster, from about 3 minutes on my Macbook down to about 25 seconds.
    
  CoderDake 
      pushed a commit
        to CoderDake/flutter
      that referenced
      this pull request
    
      Dec 28, 2023 
    
    
      
  
    
      
    
  
…lutter#140519) Roll Flutter Engine from c70f0a495ace to 1b1b2a12a597 (32 revisions) flutter/engine@c70f0a4...1b1b2a1 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 4a10533a4dc8 to bcf68d22f0fa (1 revision) (flutter/engine#49329) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 1d0c3ecd1349 to 4a10533a4dc8 (1 revision) (flutter/engine#49326) 2023-12-21 flar@google.com Ensure sorted rects in ui.Canvas for legacy compatibility (flutter/engine#49309) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 63a452b45026 to 1d0c3ecd1349 (1 revision) (flutter/engine#49318) 2023-12-21 dnfield@google.com [Impeller] Make IPLR files multi-platform (flutter/engine#49253) 2023-12-21 ditman@gmail.com [web] Defer injection of platform views until needed. (flutter/engine#48960) 2023-12-21 skia-flutter-autoroll@skia.org Roll Dart SDK from 1aef027ec953 to 63a452b45026 (2 revisions) (flutter/engine#49311) 2023-12-21 skia-flutter-autoroll@skia.org Roll Skia from 29917d8c97ca to 4b16117e94b2 (4 revisions) (flutter/engine#49310) 2023-12-21 737941+loic-sharma@users.noreply.github.com Reland "[Windows] Move to FlutterCompositor for rendering" (flutter/engine#49262) 2023-12-21 30870216+gaaclarke@users.noreply.github.com Reland `[Impeller] new blur: refactored math and fixed expanded padding size` (flutter/engine#49302) 2023-12-20 dkwingsmt@users.noreply.github.com Multiview pipeline Pt. 1: Skip illegal render calls (flutter/engine#49266) 2023-12-20 barpac02@gmail.com SemanticsUpdateBuilder: make all args non-null (flutter/engine#49148) 2023-12-20 30870216+gaaclarke@users.noreply.github.com [Impeller] fixed Rect::Contains (flutter/engine#49294) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 97c3b7e1885a to 1aef027ec953 (1 revision) (flutter/engine#49295) 2023-12-20 30870216+gaaclarke@users.noreply.github.com Revert "[Impeller] new blur: refactored math and fixed expanded padding size" (flutter/engine#49298) 2023-12-20 30870216+gaaclarke@users.noreply.github.com [Impeller] new blur: refactored math and fixed expanded padding size (flutter/engine#49206) 2023-12-20 dkwingsmt@users.noreply.github.com Multi-view pointer event (flutter/engine#46213) 2023-12-20 1961493+harryterkelsen@users.noreply.github.com [web:multiview] Only call `Renderer.clearFragmentProgramCache` on hot restart (flutter/engine#48758) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from 9cb1bb1164ea to 29917d8c97ca (1 revision) (flutter/engine#49289) 2023-12-20 bdero@google.com [Impeller] Add interactive Blur+Clip AiksTest. (flutter/engine#49283) 2023-12-20 sergiy.dubovik@supercell.com [macos] FlutterKeyboardManager memory leak fix (flutter/engine#48824) 2023-12-20 zanderso@users.noreply.github.com Don't guard Windows arm64 Dart SDK download on the release candidate flag (flutter/engine#49244) 2023-12-20 15619084+vashworth@users.noreply.github.com Fix testAppExtensionLaunching for Xcode 15/iOS 17 (flutter/engine#49242) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from 8060d6b36066 to 9cb1bb1164ea (2 revisions) (flutter/engine#49288) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from ed415d966d8a to 97c3b7e1885a (1 revision) (flutter/engine#49287) 2023-12-20 skia-flutter-autoroll@skia.org Roll Skia from d0f09ad481f7 to 8060d6b36066 (1 revision) (flutter/engine#49285) 2023-12-20 kevinjchisholm@gmail.com [release] Update release config (flutter/engine#49254) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 1732c4c92ccd to ed415d966d8a (1 revision) (flutter/engine#49274) 2023-12-20 skia-flutter-autoroll@skia.org Roll Dart SDK from 4c59838945d9 to 1732c4c92ccd (1 revision) (flutter/engine#49269) 2023-12-20 goderbauer@google.com Sync lints with flutter/flutter (flutter/engine#49192) 2023-12-19 1961493+harryterkelsen@users.noreply.github.com [web] Enforce onDrawFrame/onBeginFrame render rule (flutter/engine#49214) 2023-12-19 barpac02@gmail.com [Docs] Add more info about running tests on iOS (flutter/engine#48859) 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 jimgraham@google.com,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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...
  
      Sign up for free
      to subscribe to this conversation on GitHub.
      Already have an account?
      Sign in.
  
      Labels
      
    autosubmit
  Merge PR when tree becomes green via auto submit App 
  
    platform-web
  Code specifically for the web engine 
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
Also update tests to conform to the render rule.
The render rule is that the call to
rendershould only occur in the scope of anonDrawFrameoronBeginFramecall. As a side effect, this change also fixes a memory leak which occurs in tests using FakeAsync in theflutter_testenvironment (because they don't render anymore).Fixes flutter/flutter#137073
Fixes flutter/flutter#137669
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.