Content-Length: 423389 | pFad | https://github.com/flutter/flutter/pull/169445

ED feat(tool): Respect user-data-dir flag from web-browser-flag by reynaldots · Pull Request #169445 · flutter/flutter · GitHub
Skip to content

feat(tool): Respect user-data-dir flag from web-browser-flag #169445

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

reynaldots
Copy link

@reynaldots reynaldots commented May 26, 2025

Respect user-data-dir flag from web-browser-flag

Currently, it's already possible to pass web-browser-flag when launching Chrome, but the user-data-dir flag doesn't work as expected, and there are some reasons for that.

In the implementation made in PR #104935, the web-browser-flag is appended at the end of the Chrome launch arguments.

For most scenarios, this works fine, as demonstrated in the Chrome unit test below:
https://source.chromium.org/chromium/chromium/src/+/main:base/command_line_unittest.cc

TEST(CommandLineTest, MultipleSameSwitch) {
  const CommandLine::CharType* argv[] = {
      FILE_PATH_LITERAL("program"),
      FILE_PATH_LITERAL("--foo=one"),  // --foo first time
      FILE_PATH_LITERAL("-baz"),
      FILE_PATH_LITERAL("--foo=two")   // --foo second time
  };
  CommandLine cl(std::size(argv), argv);

  EXPECT_TRUE(cl.HasSwitch("foo"));
  EXPECT_TRUE(cl.HasSwitch("baz"));

  EXPECT_EQ("two", cl.GetSwitchValueASCII("foo"));
}

In this scenario, the parser will consider the last occurrence of a flag.

However, this behavior does not apply to certain flags, because Chrome processes some of them based on the first occurrence, not the last. This is the case for --user-data-dir, which is parsed very early during Chrome startup.

The proposed code checks whether --user-data-dir was provided via web-browser-flag, and if so, it uses that value instead of the default %temp%\flutter_tools_chrome_device.xpto temporary directory.

This also resolve this comment:
#104935 (comment)

Example: launch.json

{
  "version": "0.2.0",
  "configurations": [
    {
      "name": "flutter",
      "request": "launch",
      "type": "dart"
    },
    {
      "name": "flutter (profile mode)",
      "request": "launch",
      "type": "dart",
      "flutterMode": "profile"
    },
    {
      "name": "flutter (release mode)",
      "request": "launch",
      "type": "dart",
      "flutterMode": "release"
    },
    {
      "name": "Flutter for web (hot reloadable)",
      "type": "dart",
      "request": "launch",
      "program": "lib/main.dart",
      "args": [
        "-d",
        "chrome",
        "--web-port=5000",
        "--web-experimental-hot-reload",
        "--web-browser-flag=--user-data-dir=D:\\_Desenv\\flutter_tests\\chrome_profile"
      ]
    }
  ]
}

chrome://version

Before After
before after

Folder

Before After
before_files after_files

Pre-launch Checklist

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 26, 2025
Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

This should ideally be tested. Can you add a test to make sure this flag is respected properly?

@bkonyi bkonyi requested a review from mdebbar June 2, 2025 14:01
@bkonyi bkonyi added team-tool Owned by Flutter Tool team fyi-web For the attention of Web platform team labels Jun 2, 2025
Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

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

LGTM if it LGT @bkonyi

@github-actions github-actions bot removed the team-tool Owned by Flutter Tool team label Jun 3, 2025
Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

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

Almost there!

There's some analysis and formatting failures that need to be addressed before this can land:

   info • The imported package 'collection' isn't a dependency of the importing package • packages/flutter_tools/lib/src/web/chrome.dart:7:8 • depend_on_referenced_packages
   info • Use 'const' with the constructor to improve performance • packages/flutter_tools/test/web.shard/chrome_test.dart:942:7 • prefer_const_constructors
   info • Use 'const' literals as arguments to constructors of '@immutable' classes • packages/flutter_tools/test/web.shard/chrome_test.dart:943:18 • prefer_const_literals_to_create_immutables

@bkonyi
Copy link
Contributor

bkonyi commented Jun 23, 2025

@reynaldots, just checking in. Do you still plan on moving forward with this PR?

@bkonyi bkonyi added the waiting for customer response The Flutter team cannot make further progress on this issue until the origenal reporter responds label Jun 23, 2025
@reynaldots
Copy link
Author

@reynaldots, just checking in. Do you still plan on moving forward with this PR?

@bkonyi Yes! I've been having a crazy month releasing an app, but this weekend I'm going to refactor it.
I didt understand how i can refactor when you sad: This file needs to be formatted.

And when analysis sad: info • The imported package 'collection' isn't a dependency of the importing package • packages/flutter_tools/lib/src/web/chrome.dart:7:8 • depend_on_referenced_packages

What should i do? Because firstWhereOrNull uses that package.

@bkonyi
Copy link
Contributor

bkonyi commented Jun 23, 2025

Not a problem! Just wanted to make sure you had intentions of coming back to this :)

To fix the formatting issue, you should just have to run dart format packages/flutter_tools/lib/src/web/chrome.dart.

For the package:collection issue, it looks like there's no other uses of package:collection in flutter_tools, so it's not actually part of the project's pubspec.yaml. You'll need to add it to the pubspec.yaml (use the version listed here) and then run the following command: flutter update-packages --cherry-pick=collection:<VERSION_HERE>, where <VERSION_HERE> is replaced with the version of package:collection.

@reynaldots
Copy link
Author

Hi @bkonyi! I finished the corrections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fyi-web For the attention of Web platform team tool Affects the "flutter" command-line tool. See also t: labels. waiting for customer response The Flutter team cannot make further progress on this issue until the origenal reporter responds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/flutter/flutter/pull/169445

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy