Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

Description

Fixes chrome launching on windows by removing runInShell. Places the user data directory under .dart_tool and removes code that deletes it on tear down.

Fixes #39213
Fixes #38537

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 29, 2019
@jonahwilliams jonahwilliams requested a review from yjbanov August 29, 2019 03:26
@jonahwilliams jonahwilliams added the platform-web Web applications specifically label Aug 29, 2019
@jonahwilliams jonahwilliams requested a review from mdebbar August 29, 2019 03:26
@codecov
Copy link

codecov bot commented Aug 29, 2019

Codecov Report

Merging #39462 into master will increase coverage by 0.47%.
The diff coverage is 53.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #39462      +/-   ##
==========================================
+ Coverage   56.79%   57.26%   +0.47%     
==========================================
  Files         195      195              
  Lines       18506    18506              
==========================================
+ Hits        10510    10597      +87     
+ Misses       7996     7909      -87
Flag Coverage Δ
#flutter_tool 57.26% <53.84%> (+0.47%) ⬆️
Impacted Files Coverage Δ
packages/flutter_tools/lib/src/web/chrome.dart 50.74% <53.84%> (+41.79%) ⬆️
...utter_tools/lib/src/macos/cocoapods_validator.dart 0% <0%> (-81.82%) ⬇️
...ages/flutter_tools/lib/src/base/user_messages.dart 44.33% <0%> (-4.72%) ⬇️
packages/flutter_tools/lib/src/version.dart 90.9% <0%> (-1.92%) ⬇️
packages/flutter_tools/lib/src/cache.dart 43.02% <0%> (-0.73%) ⬇️
packages/flutter_tools/lib/src/vmservice.dart 37.3% <0%> (-0.16%) ⬇️
.../flutter_tools/lib/src/runner/flutter_command.dart 79.82% <0%> (+0.43%) ⬆️
packages/flutter_tools/lib/src/base/process.dart 86.45% <0%> (+0.64%) ⬆️
packages/flutter_tools/lib/src/context_runner.dart 70.58% <0%> (+1.96%) ⬆️
...ckages/flutter_tools/lib/src/flutter_manifest.dart 83.33% <0%> (+4.16%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f4ab27...15dd784. Read the comment docs.

Future<Chrome> launch(String url, { bool headless = false, bool skipCheck = false }) async {
final String chromeExecutable = findChromeExecutable();
final Directory dataDir = fs.systemTempDirectory.createTempSync();
final Directory dataDir = fs.directory('.dart_tool');
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be something more specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it fills up the folder with junk, will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@jonahwilliams
Copy link
Contributor Author

Ignoring add2app-macos which was disabled for repeated failures. Will land once build goes green

@jonahwilliams jonahwilliams merged commit 359b532 into flutter:master Aug 30, 2019
@jonahwilliams jonahwilliams deleted the fix_windows_chrome_launch branch August 30, 2019 02:35
await chrome.chromeConnection.getTabs();
} catch (e) {
await chrome.close();
print('here');
Copy link
Member

Choose a reason for hiding this comment

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

stray debug print

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in #39751

}));
}

class MockProcessManager extends Mock implements ProcessManager {}
Copy link
Member

Choose a reason for hiding this comment

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

The MockProcessManager defined in mocks.dart looks like it does what you want.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

platform-web Web applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flutter Web build crashed. Unable to start build daemon Enable having chrome launches persist the window locations between runs

5 participants