Skip to content
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

chore: bump chromium to 0239d631f03d0111f77b0b98c7765 (master) #20285

Merged
merged 15 commits into from Sep 21, 2019

Conversation

@electron-bot
Copy link
Collaborator

commented Sep 19, 2019

Updating Chromium to 0239d631f03d0111f77b0b98c7765 (lkgr).

See all changes in 4bb71328690460ddd3d48ef3b1eca..0239d631f03d0111f77b0b98c7765

Notes: no-notes

@electron-bot electron-bot requested a review from electron/wg-upgrades as a code owner Sep 19, 2019
@electron-bot electron-bot changed the title chore: bump chromium to 4cca4e9ac39f17b89df4af745ff80 (master) chore: bump chromium to 09f4353bc693623bf58820adf3918 (master) Sep 20, 2019
Date: Fri, 20 Sep 2019 22:03:03 +0000
Subject: {std => absl}::string_view

Introduced in https://chromium-review.googlesource.com/c/chromium/src/+/1786449, doesn't exist in MSSTL

This comment has been minimized.

Copy link
Member

left a comment

LGTM, with minor style change.

@@ -90,6 +90,7 @@ void AtomDownloadManagerDelegate::OnDownloadPathGenerated(
const content::DownloadTargetCallback& callback,
const base::FilePath& default_path) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
base::ThreadRestrictions::ScopedAllowIO allow_io;

This comment has been minimized.

Copy link
@deepak1556

deepak1556 Sep 21, 2019

Member

Which operation in this function required this ? The method is already async, it would be better to move that IO operation to a task runner that supported IO and use the response for further processing rather than scoping IO here.

This comment has been minimized.

Copy link
@nornagon

nornagon Sep 21, 2019

Member

This was the stack trace:

[...]
#4 0x55c7fda15428 base::DirectoryExists()
#5 0x55c7faa39243 file_dialog::(anonymous namespace)::FileChooserDialog::FileChooserDialog()
#6 0x55c7faa399a2 file_dialog::ShowSaveDialog()
#7 0x55c7fa9fe197 electron::AtomDownloadManagerDelegate::OnDownloadPathGenerated()
[...]

I'm all for moving this to a different thread where IO is allowed, but I didn't want that work to block this merge.

Copy link
Contributor

left a comment

LGTM

@electron-bot electron-bot changed the title chore: bump chromium to 09f4353bc693623bf58820adf3918 (master) chore: bump chromium to 0239d631f03d0111f77b0b98c7765 (master) Sep 21, 2019
@nornagon

This comment has been minimized.

Copy link
Member

commented Sep 21, 2019

Reverting the latest roller patch & merging.

…7b0b98c7765"

This reverts commit b9e223b.
@nornagon nornagon merged commit 913ec1e into master Sep 21, 2019
11 of 14 checks passed
11 of 14 checks passed
appveyor: win-ia32-testing Waiting for AppVeyor build to complete
Details
appveyor: win-woa-testing Waiting for AppVeyor build to complete
Details
appveyor: win-x64-testing Waiting for AppVeyor build to complete
Details
Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190921.2 succeeded
Details
electron-arm64-testing Build #20190921.2 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Sep 21, 2019

No Release Notes

@nornagon nornagon deleted the roller/chromium/master branch Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.