External view embedder can tell if embedded views have mutated #9653

iskakaushik · 2019-07-02T23:38:33Z

No description provided.

…s_mutated

cyanglaz

LGTM overall, left some comments.

cyanglaz · 2019-07-03T00:31:59Z

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

+  if (!views_to_recomposite_.empty()) {
+    return true;
+  }
+  if (active_composition_order_.size() != composition_order_.size()) {


nit: can we just simply compare if (active_composition_order_ != composition_order_) return true; I might be missing something in c++

cyanglaz · 2019-07-03T00:39:15Z

flow/embedded_views.h

@@ -183,6 +183,8 @@ class ExternalViewEmbedder {
 public:
  ExternalViewEmbedder() = default;

+  virtual bool HaveEmbeddedViewsMutated() = 0;


nit: maybe name it EmbeddedViewsWillMutateAtTheCurrentFrame? Since this method has to be called before the mutation actually happens at the same frame, we probably want to to have the name indicate that. This method is not going to give the desired result if called after CompositeEmbeddedView. Although EmbeddedViewsWillMutateAtTheCurrentFrame is too wordy.
I'm not good with names tho @amirh might have some thoughts.
Also, maybe document it somewhere to explain when the method should be invoked to get the desired result.

Or maybe just EmbeddedViewsWillMutate and document explain the current frame part.

I am not very satisfied with the name. having CurrentFrame is misleading as we could process multiple layer trees in the current frame. I added a comment saying that this needs to be called after pre-roll.

maybe: HasPendingViewOperations?

amirh · 2019-07-03T00:58:47Z

flow/embedded_views.h

@@ -183,6 +183,8 @@ class ExternalViewEmbedder {
 public:
  ExternalViewEmbedder() = default;

+  virtual bool HaveEmbeddedViewsMutated() = 0;


maybe: HasPendingViewOperations?

amirh · 2019-07-03T01:00:19Z

flow/embedded_views.h

@@ -183,6 +183,10 @@ class ExternalViewEmbedder {
 public:
  ExternalViewEmbedder() = default;

+  // This will return true after pre-roll if any of the embedded views


Doesn't have to be in this PR, but we will need a way to clear active_composition_order_ if we're redoing the frame, maybe by adding something like CancelFrame here?

Yup, will do it as a follow-up PR.

flutter/engine#9653)

flutter/engine@7d3e722...3c51a7b git log 7d3e722..3c51a7b --no-merges --oneline 3c51a7b Roll src/third_party/skia 11eb847a2080..857c9f955edb (2 commits) (flutter/engine#9676) 7f828dd Raster now returns an enum rather than boolean (flutter/engine#9661) 11b6afe Roll src/third_party/dart 67ab3be10d...b5aeaa6796 (flutter/engine#9675) 3c4dbe2 Revert " Roll src/third_party/dart 67ab3be10d...43891316ca (#9670)" (flutter/engine#9673) 5e596f2 Roll src/third_party/dart 67ab3be10d...43891316ca (flutter/engine#9670) 46a2239 Roll src/third_party/skia 5b52c52141ac..11eb847a2080 (6 commits) (flutter/engine#9671) b84f89b Allow embedders to add callbacks for responses to platform messages from the framework. (flutter/engine#9655) 8dac2e9 Begin separating macOS engine from view controller (flutter/engine#9654) d3616c7 Roll src/third_party/skia 0e0113dcbd9a..5b52c52141ac (8 commits) (flutter/engine#9665) cea2c36 Mutators Stack refactoring (flutter/engine#9663) 6a8782f Roll src/third_party/skia 93eeff578b08..0e0113dcbd9a (9 commits) (flutter/engine#9662) 791143f ExternalViewEmbedder can CancelFrame after pre-roll (flutter/engine#9660) d637f29 External view embedder can tell if embedded views have mutated (flutter/engine#9653) ceee3d7 Roll src/third_party/skia 38ae3f42fec1..93eeff578b08 (1 commits) (flutter/engine#9659) d757290 Roll src/third_party/skia febc162c7898..38ae3f42fec1 (3 commits) (flutter/engine#9658) 58133ab Roll src/third_party/skia 3de5c6388142..febc162c7898 (2 commits) (flutter/engine#9657) 29342dd Roll src/third_party/skia 1e2cb444e0c1..3de5c6388142 (8 commits) (flutter/engine#9656) b547356 Pipeline allows continuations that can produce to front (flutter/engine#9652) 8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651) 511b9f2 Roll src/third_party/skia 215ff3325230..1e2cb444e0c1 (12 commits) (flutter/engine#9650) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.

flutter/engine@7d3e722...3c51a7b git log 7d3e722..3c51a7b --no-merges --oneline 3c51a7b Roll src/third_party/skia 11eb847a2080..857c9f955edb (2 commits) (flutter/engine#9676) 7f828dd Raster now returns an enum rather than boolean (flutter/engine#9661) 11b6afe Roll src/third_party/dart 67ab3be10d...b5aeaa6796 (flutter/engine#9675) 3c4dbe2 Revert &flutter#34; Roll src/third_party/dart 67ab3be10d...43891316ca (flutter#9670)&flutter#34; (flutter/engine#9673) 5e596f2 Roll src/third_party/dart 67ab3be10d...43891316ca (flutter/engine#9670) 46a2239 Roll src/third_party/skia 5b52c52141ac..11eb847a2080 (6 commits) (flutter/engine#9671) b84f89b Allow embedders to add callbacks for responses to platform messages from the framework. (flutter/engine#9655) 8dac2e9 Begin separating macOS engine from view controller (flutter/engine#9654) d3616c7 Roll src/third_party/skia 0e0113dcbd9a..5b52c52141ac (8 commits) (flutter/engine#9665) cea2c36 Mutators Stack refactoring (flutter/engine#9663) 6a8782f Roll src/third_party/skia 93eeff578b08..0e0113dcbd9a (9 commits) (flutter/engine#9662) 791143f ExternalViewEmbedder can CancelFrame after pre-roll (flutter/engine#9660) d637f29 External view embedder can tell if embedded views have mutated (flutter/engine#9653) ceee3d7 Roll src/third_party/skia 38ae3f42fec1..93eeff578b08 (1 commits) (flutter/engine#9659) d757290 Roll src/third_party/skia febc162c7898..38ae3f42fec1 (3 commits) (flutter/engine#9658) 58133ab Roll src/third_party/skia 3de5c6388142..febc162c7898 (2 commits) (flutter/engine#9657) 29342dd Roll src/third_party/skia 1e2cb444e0c1..3de5c6388142 (8 commits) (flutter/engine#9656) b547356 Pipeline allows continuations that can produce to front (flutter/engine#9652) 8306ee6 Move the mutators stack handling to preroll (flutter/engine#9651) 511b9f2 Roll src/third_party/skia 215ff3325230..1e2cb444e0c1 (12 commits) (flutter/engine#9650) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.

Chris Yang and others added 6 commits July 2, 2019 14:27

refactoring to move the mutator stack handling to preroll

cb61803

merge

d1d557c

more review fixes

cd048b3

Merge branch 'master' of github.com:flutter/engine into pr/9651

682dfb5

Add support for external view embedded to know if it has changed

c14c832

Merge branch 'master' of github.com:flutter/engine into embedded_view…

22c5eb7

…s_mutated

googlebot added the cla: yes label Jul 2, 2019

iskakaushik requested review from amirh and cyanglaz July 2, 2019 23:39

iskakaushik changed the title ~~External view embedded can tell if embedded views have mutated~~ External view embedded can tell if embedder views have mutated Jul 2, 2019

iskakaushik changed the title ~~External view embedded can tell if embedder views have mutated~~ External view embedder can tell if embedded views have mutated Jul 2, 2019

iskakaushik force-pushed the embedded_views_mutated branch from 69ea711 to 5700290 Compare July 2, 2019 23:49

remove the need to reset

556d83f

iskakaushik force-pushed the embedded_views_mutated branch from 5700290 to 556d83f Compare July 2, 2019 23:51

iskakaushik mentioned this pull request Jul 3, 2019

Pipeline allows continuations that can produce to front #9652

Merged

cyanglaz reviewed Jul 3, 2019

View reviewed changes

address comments

de884bc

amirh reviewed Jul 3, 2019

View reviewed changes

Rename to HasPendingViewOperations

49c7eef

cyanglaz approved these changes Jul 3, 2019

View reviewed changes

iskakaushik merged commit d637f29 into flutter:master Jul 3, 2019

iskakaushik deleted the embedded_views_mutated branch July 3, 2019 14:28

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..d637f291a96f (8 commits) flutter/flutter#35533

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2019

d637f29 External view embedder can tell if embedded views have mutated (

398a851

flutter/engine#9653)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2019

d637f29 External view embedder can tell if embedded views have mutated (

1526175

flutter/engine#9653)

This was referenced Jul 3, 2019

Roll engine 7d3e722349e7..791143f17281 (9 commits) flutter/flutter#35538

Closed

Roll engine 7d3e722349e7..6a8782fe0162 (10 commits) flutter/flutter#35540

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2019

d637f29 External view embedder can tell if embedded views have mutated (

7ae34bd

flutter/engine#9653)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..cea2c3617f61 (11 commits) flutter/flutter#35546

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2019

d637f29 External view embedder can tell if embedded views have mutated (

f40f8f3

flutter/engine#9653)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2019

d637f29 External view embedder can tell if embedded views have mutated (

4c4de49

flutter/engine#9653)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..d3616c7a31de (12 commits) flutter/flutter#35552

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2019

d637f29 External view embedder can tell if embedded views have mutated (

00dadd1

flutter/engine#9653)

engine-flutter-autoroll mentioned this pull request Jul 3, 2019

Roll engine 7d3e722349e7..8dac2e979697 (13 commits) flutter/flutter#35553

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2019

d637f29 External view embedder can tell if embedded views have mutated (

ef4d1ea

flutter/engine#9653)

This was referenced Jul 3, 2019

Roll engine 7d3e722349e7..b84f89b44b75 (14 commits) flutter/flutter#35557

Closed

Roll engine 7d3e722349e7..46a2239b4240 (15 commits) flutter/flutter#35562

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2019

d637f29 External view embedder can tell if embedded views have mutated (

d7a89d2

flutter/engine#9653)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2019

d637f29 External view embedder can tell if embedded views have mutated (

a80e27f

flutter/engine#9653)

This was referenced Jul 4, 2019

Roll engine 7d3e722349e7..3c4dbe223cfe (17 commits) flutter/flutter#35565

Closed

Roll engine 7d3e722349e7..11b6afe00ca5 (18 commits) flutter/flutter#35569

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2019

d637f29 External view embedder can tell if embedded views have mutated (

322922c

flutter/engine#9653)

engine-flutter-autoroll mentioned this pull request Jul 4, 2019

Roll engine 7d3e722349e7..7f828dd3e3d5 (19 commits) flutter/flutter#35571

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2019

d637f29 External view embedder can tell if embedded views have mutated (

1b90ee9

flutter/engine#9653)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2019

d637f29 External view embedder can tell if embedded views have mutated (

d3b1bdc

flutter/engine#9653)

engine-flutter-autoroll mentioned this pull request Jul 4, 2019

Roll engine 7d3e722349e7..3c51a7bfff77 (20 commits) flutter/flutter#35572

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

External view embedder can tell if embedded views have mutated #9653

External view embedder can tell if embedded views have mutated #9653

iskakaushik commented Jul 2, 2019

cyanglaz left a comment

cyanglaz Jul 3, 2019

iskakaushik Jul 3, 2019

cyanglaz Jul 3, 2019 •

edited

Loading

cyanglaz Jul 3, 2019

iskakaushik Jul 3, 2019

amirh Jul 3, 2019

iskakaushik Jul 3, 2019

amirh Jul 3, 2019

amirh Jul 3, 2019

iskakaushik Jul 3, 2019

External view embedder can tell if embedded views have mutated #9653

External view embedder can tell if embedded views have mutated #9653

Conversation

iskakaushik commented Jul 2, 2019

cyanglaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyanglaz Jul 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyanglaz Jul 3, 2019 •

edited

Loading