Accept file URIs as parameters in the RunInView service RPC #6191

jason-simmons · 2018-09-06T22:39:59Z

jason-simmons · 2018-09-06T22:40:15Z

liyuqian · 2018-09-06T22:45:33Z

fml/paths.cc

  return stream.str();
 }

+std::string SanitizeURIEscapedCharacters(const std::string& str) {


I feel that this function is a nice candidate for a unit test which both helps ensure correctness, and helps the reader understand what the function is doing. Do we have unit tests for fml yet?

aam · 2018-09-06T22:47:09Z

fml/platform/win/paths_win.cc

+    return uri;
+
+  std::string file_path = uri.substr(kFileURLPrefixLength);
+  std::replace(file_path.begin(), file_path.end(), '/', '\\');


msvc complains:

ninja -t msvc -e environment.x64 -- C:\src\goma/gomacc.exe "C:\src\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\VC\Tools\MSVC\14.11.25503\bin\HostX64\x64/cl.exe" /nologo /showIncludes /FC @obj/flutter/fml/platform/win/fml.paths_win.obj.rsp /c ../../flutter/fml/platform/win/paths_win.cc /Foobj/flutter/fml/platform/win/fml.paths_win.obj /Fdobj/flutter/fml/fml_cc.pdb c:\src\flutter\engine\src\flutter\fml\platform\win\paths_win.cc(88): error C2039: 'replace': is not a member of 'std' c:\src\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\vc\tools\msvc\14.11.25503\include\string(15): note: see declaration of 'std' c:\src\flutter\engine\src\flutter\fml\platform\win\paths_win.cc(88): error C3861: 'replace': identifier not found

string::replace / std::replace_copy?

added #include <algorithm>

cbracken · 2018-09-06T22:45:01Z

fml/platform/posix/paths_posix.cc


 namespace {

+const char kFileURLPrefix[] = "file://";


Here and below: not that it matters in the context, but I imagine this could be constexpr.

liyuqian

LGTM

aam · 2018-09-06T23:56:54Z

Thanks!

aam · 2018-09-06T23:57:42Z

Should be able to land change to flutter tools to switch to uris:

diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart
index 51bc7fae6..688982e64 100644
--- a/packages/flutter_tools/lib/src/vmservice.dart
+++ b/packages/flutter_tools/lib/src/vmservice.dart
@@ -932,8 +932,8 @@ class VM extends ServiceObjectOwner {
     return invokeRpc<ServiceMap>('_flutter.runInView',
       params: <String, dynamic> {
         'viewId': viewId,
-        'mainScript': main.toFilePath(windows: false),
-        'packagesFile': packages.toFilePath(windows: false),
+        'mainScript': main.toString(),
+        'packagesFile': packages.toString(),
         'assetDirectory': assetsDirectory.toFilePath(windows: false)
     });
   }

See flutter/flutter#21348

flutter/engine@58a1894...85d47fb git log 58a1894..85d47fb --no-merges --oneline 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@58a1894...687cf08 git log 58a1894..687cf08 --no-merges --oneline 687cf08 Add option to invert paint colors to be used for smart invert accessibility on iOS (flutter/engine#6176) 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@58a1894...2af72eb git log 58a1894..2af72eb --no-merges --oneline 2af72eb Don't map iOS reduce motion to disabled animations (flutter/engine#6194) 687cf08 Add option to invert paint colors to be used for smart invert accessibility on iOS (flutter/engine#6176) 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@58a1894...16c56af git log 58a1894..16c56af --no-merges --oneline 16c56af Roll src/third_party/skia 300b6197e344..094b3eaf479c (17 commits) (flutter/engine#6200) 2af72eb Don't map iOS reduce motion to disabled animations (flutter/engine#6194) 687cf08 Add option to invert paint colors to be used for smart invert accessibility on iOS (flutter/engine#6176) 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@58a1894...93dac2a git log 58a1894..93dac2a --no-merges --oneline 93dac2a Always save canvas and correctly pass antialias boolean in ClipRects. (flutter/engine#6199) 16c56af Roll src/third_party/skia 300b6197e344..094b3eaf479c (17 commits) (flutter/engine#6200) 2af72eb Don't map iOS reduce motion to disabled animations (flutter/engine#6194) 687cf08 Add option to invert paint colors to be used for smart invert accessibility on iOS (flutter/engine#6176) 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@58a1894...e27a2e9 git log 58a1894..e27a2e9 --no-merges --oneline e27a2e9 Dart SDK roll for 2018/09/07 (flutter/engine#6201) 93dac2a Always save canvas and correctly pass antialias boolean in ClipRects. (flutter/engine#6199) 16c56af Roll src/third_party/skia 300b6197e344..094b3eaf479c (17 commits) (flutter/engine#6200) 2af72eb Don't map iOS reduce motion to disabled animations (flutter/engine#6194) 687cf08 Add option to invert paint colors to be used for smart invert accessibility on iOS (flutter/engine#6176) 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@58a1894...bf96dbe git log 58a1894..bf96dbe --no-merges --oneline bf96dbe Revert "Some cleanups enabled by removing support for Dart. (flutter#5621)" (flutter/engine#6205) 3358115 Roll src/third_party/skia 094b3eaf479c..2810c856dfa2 (6 commits) (flutter/engine#6203) 4c2448d Some cleanups enabled by removing support for Dart. (flutter/engine#5621) e27a2e9 Dart SDK roll for 2018/09/07 (flutter/engine#6201) 93dac2a Always save canvas and correctly pass antialias boolean in ClipRects. (flutter/engine#6199) 16c56af Roll src/third_party/skia 300b6197e344..094b3eaf479c (17 commits) (flutter/engine#6200) 2af72eb Don't map iOS reduce motion to disabled animations (flutter/engine#6194) 687cf08 Add option to invert paint colors to be used for smart invert accessibility on iOS (flutter/engine#6176) 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@58a1894...b952331 git log 58a1894..b952331 --no-merges --oneline b952331 Allow embedders to specify a custom GL proc address resolver. (flutter/engine#6204) bf96dbe Revert "Some cleanups enabled by removing support for Dart. (flutter#5621)" (flutter/engine#6205) 3358115 Roll src/third_party/skia 094b3eaf479c..2810c856dfa2 (6 commits) (flutter/engine#6203) 4c2448d Some cleanups enabled by removing support for Dart. (flutter/engine#5621) e27a2e9 Dart SDK roll for 2018/09/07 (flutter/engine#6201) 93dac2a Always save canvas and correctly pass antialias boolean in ClipRects. (flutter/engine#6199) 16c56af Roll src/third_party/skia 300b6197e344..094b3eaf479c (17 commits) (flutter/engine#6200) 2af72eb Don't map iOS reduce motion to disabled animations (flutter/engine#6194) 687cf08 Add option to invert paint colors to be used for smart invert accessibility on iOS (flutter/engine#6176) 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

flutter/engine@58a1894...bf3c224 git log 58a1894..bf3c224 --no-merges --oneline bf3c224 Roll src/third_party/skia 2810c856dfa2..a3dc329d1db1 (1 commits) (flutter/engine#6206) b952331 Allow embedders to specify a custom GL proc address resolver. (flutter/engine#6204) bf96dbe Revert "Some cleanups enabled by removing support for Dart. (flutter#5621)" (flutter/engine#6205) 3358115 Roll src/third_party/skia 094b3eaf479c..2810c856dfa2 (6 commits) (flutter/engine#6203) 4c2448d Some cleanups enabled by removing support for Dart. (flutter/engine#5621) e27a2e9 Dart SDK roll for 2018/09/07 (flutter/engine#6201) 93dac2a Always save canvas and correctly pass antialias boolean in ClipRects. (flutter/engine#6199) 16c56af Roll src/third_party/skia 300b6197e344..094b3eaf479c (17 commits) (flutter/engine#6200) 2af72eb Don't map iOS reduce motion to disabled animations (flutter/engine#6194) 687cf08 Add option to invert paint colors to be used for smart invert accessibility on iOS (flutter/engine#6176) 85d47fb Accept file URIs as parameters in the RunInView service RPC (flutter/engine#6191) 5a19d76 Roll src/third_party/skia 0b80e62a14ae..300b6197e344 (9 commits) (flutter/engine#6198) f33615c Roll src/third_party/skia b090b2b26803..0b80e62a14ae (2 commits) (flutter/engine#6197) 75f6bdf Revert "Dart SDK roll for 2018/09/06 (flutter#6189)" (flutter/engine#6192) b09563b Roll src/third_party/skia 106d04e907c1..b090b2b26803 (1 commits) (flutter/engine#6193) e164e83 Reset the raster cache when the compositor context is created. (flutter/engine#6150) f02fc8c Dart SDK roll for 2018/09/06 (flutter/engine#6189) 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, who should be CC'd on the roll, and stop the roller if necessary.

…6191) See flutter/flutter#21348

googlebot added the cla: yes label Sep 6, 2018

liyuqian reviewed Sep 6, 2018

View reviewed changes

aam reviewed Sep 6, 2018

View reviewed changes

cbracken approved these changes Sep 6, 2018

View reviewed changes

jason-simmons force-pushed the bug_21348 branch from 8dceefd to 130cbf2 Compare September 6, 2018 23:11

liyuqian approved these changes Sep 6, 2018

View reviewed changes

aam approved these changes Sep 6, 2018

View reviewed changes

Accept file URIs as parameters in the RunInView service RPC

5f43b4a

See flutter/flutter#21348

jason-simmons force-pushed the bug_21348 branch from 130cbf2 to 5f43b4a Compare September 7, 2018 00:05

jason-simmons merged commit 85d47fb into flutter:master Sep 7, 2018

engine-flutter-autoroll mentioned this pull request Sep 7, 2018

Roll engine 58a1894a1cd7..85d47fb03c1b (7 commits) flutter/flutter#21554

Closed

engine-flutter-autoroll mentioned this pull request Sep 7, 2018

Roll engine 58a1894a1cd7..687cf08c70d5 (8 commits) flutter/flutter#21557

Closed

engine-flutter-autoroll mentioned this pull request Sep 7, 2018

Roll engine 58a1894a1cd7..2af72eb2cc05 (9 commits) flutter/flutter#21560

Closed

engine-flutter-autoroll mentioned this pull request Sep 7, 2018

Roll engine 58a1894a1cd7..16c56afb09ee (10 commits) flutter/flutter#21568

Closed

engine-flutter-autoroll mentioned this pull request Sep 7, 2018

Roll engine 58a1894a1cd7..93dac2a88d16 (11 commits) flutter/flutter#21572

Closed

engine-flutter-autoroll mentioned this pull request Sep 7, 2018

Roll engine 58a1894a1cd7..e27a2e955bc3 (12 commits) flutter/flutter#21580

Closed

engine-flutter-autoroll mentioned this pull request Sep 8, 2018

Roll engine 58a1894a1cd7..bf96dbed38ed (15 commits) flutter/flutter#21584

Closed

engine-flutter-autoroll mentioned this pull request Sep 8, 2018

Roll engine 58a1894a1cd7..b9523318caa1 (16 commits) flutter/flutter#21586

Closed

engine-flutter-autoroll mentioned this pull request Sep 8, 2018

Roll engine 58a1894a1cd7..bf3c224197cf (17 commits) flutter/flutter#21588

Closed

amirh pushed a commit to amirh/engine that referenced this pull request Sep 21, 2018

Accept file URIs as parameters in the RunInView service RPC (flutter#…

1e7fadb

…6191) See flutter/flutter#21348

amirh pushed a commit to amirh/engine that referenced this pull request Sep 21, 2018

Accept file URIs as parameters in the RunInView service RPC (flutter#…

9a72b35

…6191) See flutter/flutter#21348

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Accept file URIs as parameters in the RunInView service RPC #6191

Accept file URIs as parameters in the RunInView service RPC #6191

Uh oh!

jason-simmons commented Sep 6, 2018

Uh oh!

jason-simmons commented Sep 6, 2018

Uh oh!

liyuqian Sep 6, 2018

Uh oh!

jason-simmons Sep 6, 2018

Uh oh!

aam Sep 6, 2018

Uh oh!

cbracken Sep 6, 2018

Uh oh!

jason-simmons Sep 6, 2018

Uh oh!

cbracken Sep 6, 2018

Uh oh!

jason-simmons Sep 6, 2018

Uh oh!

liyuqian left a comment

Uh oh!

aam commented Sep 6, 2018

Uh oh!

aam commented Sep 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Accept file URIs as parameters in the RunInView service RPC #6191

Accept file URIs as parameters in the RunInView service RPC #6191

Uh oh!

Conversation

jason-simmons commented Sep 6, 2018

Uh oh!

jason-simmons commented Sep 6, 2018

Uh oh!

liyuqian Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

jason-simmons Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

aam Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

cbracken Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

jason-simmons Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

cbracken Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

jason-simmons Sep 6, 2018

Choose a reason for hiding this comment

Uh oh!

liyuqian left a comment

Choose a reason for hiding this comment

Uh oh!

aam commented Sep 6, 2018

Uh oh!

aam commented Sep 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants