[web] Environment variable to disable felt snapshot #13187

mdebbar · 2019-10-16T22:25:39Z

This PR also includes:

Fix a Chrome installation issue on Mac (after the yaml lock file format has changed).
Update README.md with instructions for doing felt tool development.

nturgut

Thanks Mouad!

nturgut · 2019-10-17T14:31:35Z

lib/web_ui/dev/felt

+  echo "[Snapshot mode: off]"
+  # Running without snapshot means there is high likelyhood of local changes. In
+  # that case, let's clear the snapshot to avoid any surprises.
+  rm -f "$SNAPSHOT_PATH"


suggestion: Shall we use "rm -rf"?

"-r" argument deletes recursively, may be we might want to use it?

$SNAPSHOT_PATH and $STAMP_PATH point to files, not directories. The recursive flag -r only has an effect on directories, right?

yjbanov · 2019-10-17T17:11:28Z

lib/web_ui/dev/felt

+    install_deps
+    mkdir -p $DART_TOOL_DIR
+
+    echo "Creating the snapshot"


This message seems redundant given the message above

yjbanov · 2019-10-17T17:11:59Z

lib/web_ui/dev/felt

+    "$DART_SDK_DIR/bin/dart" --snapshot="$SNAPSHOT_PATH" --packages="$WEB_UI_DIR/.packages" "$SCRIPT_PATH"
+    echo "$REVISION" > "$STAMP_PATH"
+  else
+    echo "[Snapshot mode: on] (using existing snapshot)"


I think the default should be silent.

I'm still debating this. On the one hand, like you said, the happy path should be silent. On the other hand, my concern is that someone would make changes in felt and forget to turn off snapshotting. Showing the message all the time makes it easier to realize that the snapshot mode is on.

I'll make it silent for now and see. If we start making a lot of mistakes, I'll "unsilent" it.

Yeah, it depends on how often we'll be making changes to felt relative to using it. I'm not terribly attached to one way or the other.

…engine#13187)

[email protected]:flutter/engine.git/compare/508146f0defb...7a621a7 git log 508146f..7a621a7 --no-merges --oneline 2019-10-18 [email protected] Roll src/third_party/skia 20eafffd2d2f..b80d31f8cbe2 (4 commits) (flutter/engine#13226) 2019-10-18 [email protected] Roll src/third_party/skia da29d70f1a59..20eafffd2d2f (1 commits) (flutter/engine#13223) 2019-10-18 [email protected] Roll src/third_party/skia 63a387395751..da29d70f1a59 (11 commits) (flutter/engine#13221) 2019-10-18 [email protected] Roll fuchsia/sdk/core/linux-amd64 from WpvU_... to _G94w... (flutter/engine#13220) 2019-10-18 [email protected] Specify a human readable reason for an error from the embedder API. (flutter/engine#13218) 2019-10-18 [email protected] Roll fuchsia/sdk/core/mac-amd64 from _9Uy_... to KNygX... (flutter/engine#13219) 2019-10-17 [email protected] Reland ICU update to 64.2 (flutter/engine#13216) 2019-10-17 [email protected] Use `window.devicePixelRatio` in the CanvasKit backend (flutter/engine#13192) 2019-10-17 [email protected] Re-enable WeakPtr ThreadChecker and fix associated failures (flutter/engine#12257) 2019-10-17 [email protected] Re-land "Custom compositor layers must take into account the device pixel ratio." 2019-10-17 [email protected] Add trace events around custom compositor callbacks. (flutter/engine#13212) 2019-10-17 [email protected] Roll src/third_party/skia 93e853bf2b83..63a387395751 (9 commits) (flutter/engine#13208) 2019-10-17 [email protected] Roll src/third_party/dart 9b3c7f64d8..a61c775db8 (5 commits) 2019-10-17 [email protected] Document //flutter/runtime/dart_snapshot.h (flutter/engine#13196) 2019-10-17 [email protected] Revert "Custom compositor layers must take into account the device pixel ratio. (#13193)" (flutter/engine#13211) 2019-10-17 [email protected] wrap the text in text editing. This was causing a missalingment issue in textarea. (flutter/engine#13207) 2019-10-17 [email protected] Custom compositor layers must take into account the device pixel ratio. (flutter/engine#13193) 2019-10-17 [email protected] [web] Environment variable to disable felt snapshot (flutter/engine#13187) 2019-10-17 [email protected] Roll src/third_party/dart 9e636b5ab4..9b3c7f64d8 (5 commits) 2019-10-17 [email protected] Roll src/third_party/skia 0df7697235b4..93e853bf2b83 (1 commits) (flutter/engine#13205) 2019-10-17 [email protected] Roll fuchsia/sdk/core/linux-amd64 from ek5iQ... to WpvU_... (flutter/engine#13203) 2019-10-17 [email protected] Roll fuchsia/sdk/core/mac-amd64 from 6j3Gw... to _9Uy_... (flutter/engine#13202) 2019-10-17 [email protected] Roll src/third_party/skia 6a19e03047cc..0df7697235b4 (1 commits) (flutter/engine#13200) 2019-10-17 [email protected] Roll src/third_party/dart 1e3e9ee04c..9e636b5ab4 (9 commits) 2019-10-17 [email protected] Roll src/third_party/skia f29cb70281d5..6a19e03047cc (5 commits) (flutter/engine#13198) 2019-10-17 [email protected] Roll src/third_party/dart f020ce5d23..1e3e9ee04c (12 commits) 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] on the revert to ensure that a human is aware of the problem. 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/+/master/autoroll/README.md

[email protected]:flutter/engine.git/compare/508146f0defb...7a621a7 git log 508146f..7a621a7 --no-merges --oneline 2019-10-18 [email protected] Roll src/third_party/skia 20eafffd2d2f..b80d31f8cbe2 (4 commits) (flutter/engine#13226) 2019-10-18 [email protected] Roll src/third_party/skia da29d70f1a59..20eafffd2d2f (1 commits) (flutter/engine#13223) 2019-10-18 [email protected] Roll src/third_party/skia 63a387395751..da29d70f1a59 (11 commits) (flutter/engine#13221) 2019-10-18 [email protected] Roll fuchsia/sdk/core/linux-amd64 from WpvU_... to _G94w... (flutter/engine#13220) 2019-10-18 [email protected] Specify a human readable reason for an error from the embedder API. (flutter/engine#13218) 2019-10-18 [email protected] Roll fuchsia/sdk/core/mac-amd64 from _9Uy_... to KNygX... (flutter/engine#13219) 2019-10-17 [email protected] Reland ICU update to 64.2 (flutter/engine#13216) 2019-10-17 [email protected] Use `window.devicePixelRatio` in the CanvasKit backend (flutter/engine#13192) 2019-10-17 [email protected] Re-enable WeakPtr ThreadChecker and fix associated failures (flutter/engine#12257) 2019-10-17 [email protected] Re-land "Custom compositor layers must take into account the device pixel ratio." 2019-10-17 [email protected] Add trace events around custom compositor callbacks. (flutter/engine#13212) 2019-10-17 [email protected] Roll src/third_party/skia 93e853bf2b83..63a387395751 (9 commits) (flutter/engine#13208) 2019-10-17 [email protected] Roll src/third_party/dart 9b3c7f64d8..a61c775db8 (5 commits) 2019-10-17 [email protected] Document //flutter/runtime/dart_snapshot.h (flutter/engine#13196) 2019-10-17 [email protected] Revert "Custom compositor layers must take into account the device pixel ratio. (flutter#13193)" (flutter/engine#13211) 2019-10-17 [email protected] wrap the text in text editing. This was causing a missalingment issue in textarea. (flutter/engine#13207) 2019-10-17 [email protected] Custom compositor layers must take into account the device pixel ratio. (flutter/engine#13193) 2019-10-17 [email protected] [web] Environment variable to disable felt snapshot (flutter/engine#13187) 2019-10-17 [email protected] Roll src/third_party/dart 9e636b5ab4..9b3c7f64d8 (5 commits) 2019-10-17 [email protected] Roll src/third_party/skia 0df7697235b4..93e853bf2b83 (1 commits) (flutter/engine#13205) 2019-10-17 [email protected] Roll fuchsia/sdk/core/linux-amd64 from ek5iQ... to WpvU_... (flutter/engine#13203) 2019-10-17 [email protected] Roll fuchsia/sdk/core/mac-amd64 from 6j3Gw... to _9Uy_... (flutter/engine#13202) 2019-10-17 [email protected] Roll src/third_party/skia 6a19e03047cc..0df7697235b4 (1 commits) (flutter/engine#13200) 2019-10-17 [email protected] Roll src/third_party/dart 1e3e9ee04c..9e636b5ab4 (9 commits) 2019-10-17 [email protected] Roll src/third_party/skia f29cb70281d5..6a19e03047cc (5 commits) (flutter/engine#13198) 2019-10-17 [email protected] Roll src/third_party/dart f020ce5d23..1e3e9ee04c (12 commits) 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] on the revert to ensure that a human is aware of the problem. 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/+/master/autoroll/README.md

[web] Environment variable to disable felt snapshot

172ae7d

mdebbar added the platform-web Code specifically for the web engine label Oct 16, 2019

mdebbar requested review from nturgut and yjbanov October 16, 2019 22:25

mdebbar self-assigned this Oct 16, 2019

googlebot added the cla: yes label Oct 16, 2019

nturgut approved these changes Oct 17, 2019

View reviewed changes

yjbanov approved these changes Oct 17, 2019

View reviewed changes

Address Yegor's comments

84cef19

mdebbar merged commit 8f621ad into flutter:master Oct 17, 2019

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

8f621ad [web] Environment variable to disable felt snapshot (flutter/…

df83bb4

…engine#13187)

engine-flutter-autoroll mentioned this pull request Oct 17, 2019

Roll engine 508146f0defb..8f621ad3c7e6 (9 commits) flutter/flutter#42944

Closed

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

8f621ad [web] Environment variable to disable felt snapshot (flutter/…

aee5893

…engine#13187)

engine-flutter-autoroll mentioned this pull request Oct 17, 2019

Roll engine 508146f0defb..8f621ad3c7e6 (9 commits) flutter/flutter#42950

Closed

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

8f621ad [web] Environment variable to disable felt snapshot (flutter/…

7fbfcd0

…engine#13187)

engine-flutter-autoroll mentioned this pull request Oct 18, 2019

Roll engine 508146f0defb..7a621a7f7e4f (26 commits) flutter/flutter#43017

Merged

mdebbar deleted the felt_snapshot branch April 15, 2021 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[web] Environment variable to disable felt snapshot #13187

[web] Environment variable to disable felt snapshot #13187

Uh oh!

mdebbar commented Oct 16, 2019

Uh oh!

nturgut left a comment

Uh oh!

nturgut Oct 17, 2019

Uh oh!

mdebbar Oct 17, 2019

Uh oh!

yjbanov Oct 17, 2019

Uh oh!

yjbanov Oct 17, 2019

Uh oh!

mdebbar Oct 17, 2019

Uh oh!

yjbanov Oct 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[web] Environment variable to disable felt snapshot #13187

[web] Environment variable to disable felt snapshot #13187

Uh oh!

Conversation

mdebbar commented Oct 16, 2019

Uh oh!

nturgut left a comment

Choose a reason for hiding this comment

Uh oh!

nturgut Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

mdebbar Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

yjbanov Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

yjbanov Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

mdebbar Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

yjbanov Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants