Skip to content

Upgrade to Chromium 62#11230

Merged
zcbenz merged 60 commits intomasterfrom
upgrade-to-chromium-62
Feb 26, 2018
Merged

Upgrade to Chromium 62#11230
zcbenz merged 60 commits intomasterfrom
upgrade-to-chromium-62

Conversation

@codebytere
Copy link
Copy Markdown
Member

@codebytere codebytere commented Nov 23, 2017 •
edited by alexeykuzmin
Loading

🙅 DO NOT MERGE.

Work on upgrading to Chromium 62 with associated updates to crashpad & node.

PR to libcc: electron/libchromiumcontent#376
Project board: https://github.com/electron/electron/projects/6

/cc @ckerr @gavignus @alexeykuzmin

@codebytere codebytere requested review from a team November 23, 2017 11:10
@alexeykuzmin alexeykuzmin force-pushed the upgrade-to-chromium-62 branch 3 times, most recently from 1ca2601 to 9f902b4 Compare November 27, 2017 12:05
zcbenz
zcbenz reviewed Nov 27, 2017
View reviewed changes
brightray/browser/url_request_context_getter.cc Outdated
std::unique_ptr<base::CallbackList<void(const CookieDetails*)>::Subscription>
URLRequestContextGetter::RegisterCookieChangeCallback(
const base::Callback<void(const CookieDetails*)>& cb) {
return cookie_change_sub_list_.Add(cb);
Copy link
Copy Markdown
Contributor

@zcbenz zcbenz Nov 27, 2017

Choose a reason for hiding this comment

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

@deepak1556 Having the Subscription destroyed in UI thread with CallbackList destroyed in IO thread could cause race conditions on exit. Using content::NotificationService could be a safer choice, like what Chromium did with chrome::NOTIFICATION_COOKIE_CHANGED_FOR_EXTENSIONS.

Copy link
Copy Markdown
Member

@deepak1556 deepak1556 Nov 27, 2017

Choose a reason for hiding this comment

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

I went down that road but soon discovered that NotificationService is being removed https://bugs.chromium.org/p/chromium/issues/detail?id=268984 . So chose this instead, moving the ownership of CallbackList to BrowserContext should fix the thread issue. Thoughts ?

Copy link
Copy Markdown
Contributor

@zcbenz zcbenz Nov 28, 2017

Choose a reason for hiding this comment

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

moving the ownership of CallbackList to BrowserContext should fix the thread issue.

This sounds good to me.

@alexeykuzmin
Copy link
Copy Markdown
Contributor

alexeykuzmin commented Nov 27, 2017

Validate transparent window patch electron/libchromiumcontent@eb9613a

@MarshallOfSound JFYI, both debug and release builds of Electron on Mac should be fine now )

@alexeykuzmin alexeykuzmin mentioned this pull request Nov 27, 2017
Merged
This was referenced Nov 29, 2017
Closed
Closed
@alexeykuzmin alexeykuzmin force-pushed the upgrade-to-chromium-62 branch from ca29a2c to fd87268 Compare December 4, 2017 21:45
@MarshallOfSound MarshallOfSound mentioned this pull request Dec 6, 2017
Closed
@zcbenz zcbenz force-pushed the upgrade-to-chromium-62 branch from 5de9acf to c95084a Compare December 8, 2017 07:39
@alexeykuzmin alexeykuzmin force-pushed the upgrade-to-chromium-62 branch from 3a049c1 to 6bd2dcd Compare December 10, 2017 23:29
@DominikLevitsky
Copy link
Copy Markdown

DominikLevitsky commented Dec 12, 2017

Any super-rough estimates on this one?

@alexeykuzmin
Copy link
Copy Markdown
Contributor

alexeykuzmin commented Dec 12, 2017

Any super-rough estimates on this one?

@DominikLevitsky estimates almost always sound like promises, and I don't want to make any.

@rmetzler rmetzler mentioned this pull request Dec 16, 2017
Merged
@alexeykuzmin alexeykuzmin force-pushed the upgrade-to-chromium-62 branch 2 times, most recently from 6cbf4ce to 54deb37 Compare December 22, 2017 06:05
@zcbenz
Copy link
Copy Markdown
Contributor

zcbenz commented Dec 27, 2017

I'm rebasing this branch and the branch in Node repo to port #11527.

@zcbenz zcbenz force-pushed the upgrade-to-chromium-62 branch from d2eb942 to 8510778 Compare December 27, 2017 13:01
@zcbenz
Copy link
Copy Markdown
Contributor

zcbenz commented Dec 27, 2017

@codebytere @alexeykuzmin I have rebased the electron-node-v9.2.1 branch to amend the "Allocate memory of Buffer with V8's allocator" commit to fix a crash in crypto module (#11527), please make sure you are including the right commit when doing Chrome upgrades.

I have also pushed the fix to electron-node-v8.2.1 and electron-node-v8.7.0 branches as extra commits, since these branches have already been referenced by the master branch of Electron.

@alexeykuzmin alexeykuzmin force-pushed the upgrade-to-chromium-62 branch from 0aa29ce to b45fb4d Compare January 5, 2018 05:47
@codebytere codebytere requested a review from a team January 6, 2018 15:58
@mividtim
Copy link
Copy Markdown

mividtim commented Jan 26, 2018

Why Node 9? The LTS version of Node is 8.9. IMHO, it would be most wise to embed the LTS version of Node in the latest Electron.

@alexeykuzmin alexeykuzmin mentioned this pull request Jan 26, 2018
Closed
alexeykuzmin and others added 19 commits February 23, 2018 10:21
@alexeykuzmin @zcbenz
Fail "navigator.serviceWorker" tests if WebContents crashes
dcb369e
@alexeykuzmin @zcbenz
Add init_webrtc lib to the list of static libraries
0ec2eba
@zcbenz
Update sysroot to Debian Stretch
da7fc54
@alexeykuzmin @zcbenz
Improve process launch handle sharing API.
f5d207f 
https://codereview.chromium.org/2950153002
@zcbenz
Update sysroot images to fix linking error
e6bad6e
@deepak1556 @zcbenz
update libcc for loading service worker under file scheme
7cad5d0
@zcbenz
Update node: Fix crash when doing crypto operation
12b37c7
@alexeykuzmin @zcbenz
Update libcc reference
31eb5e2
@codebytere @zcbenz
Upgrade to node v9.3.0 (#11507)
0e5b6f9 
* update submodule refs for node v9.3.0

* Define "llvm_version" for Node.js build

* NODE_MODULE_CONTEXT_AWARE_BUILTIN -> NODE_BUILTIN_MODULE_CONTEXT_AWARE

* update NodePlatform to MultiIsolatePlatform

* fix linting error

* update node ref

* REVIEW: Explicitly register builtin modules

nodejs/node#16565

* update libcc ref

* switch libcc to c62

* REVIEW: Address node api changes

- Always start the inspector agent for nodejs/node#17085
- Set the tracing controller for node nodejs/node#15538
- Isolate data creation now requires plaform nodejs/node#16700
@codebytere @zcbenz
update update submodule ref for updated node 9.3.0 (#11708)
653ceb9
@zcbenz
Fix linking error with atomic
b7b40d9
@zcbenz
Link with libcc's libc++ on Linux
9fec1a3
@zcbenz
Make sure --sysroot is appended at last
2fcf2c8
@zcbenz
node.so should link with libc++.so
9d05c59
@zcbenz
Fix build error after rebasing
39e0433
@zcbenz
Fix assertion when get X11 error in new thread
b25175a
@zcbenz
Fix error when cross compilation
81c23b8
@zcbenz
Disable stack dumping for 32bit ARM
b9ab2fe
@alexeykuzmin @zcbenz
Update libcc revision
7e3b690
@zcbenz zcbenz force-pushed the upgrade-to-chromium-62 branch from b12a114 to 7e3b690 Compare February 23, 2018 01:22
@zcbenz
Copy link
Copy Markdown
Contributor

zcbenz commented Feb 26, 2018

It seems that everyone agrees to merge Chrome62.

@zcbenz zcbenz merged commit f7786a9 into master Feb 26, 2018
@PalmerAL PalmerAL mentioned this pull request Mar 6, 2018
Closed
@zcbenz zcbenz deleted the upgrade-to-chromium-62 branch March 8, 2018 01:06
@DominikLevitsky
Copy link
Copy Markdown

DominikLevitsky commented Mar 19, 2018

As this seems to be merged to main, when can we expect a release with Chromium 62? Soon-ish?

@MarshallOfSound
Copy link
Copy Markdown
Member

MarshallOfSound commented Mar 19, 2018

@DominikLevitsky We don't have any release estimates at this time

@andens andens mentioned this pull request Mar 20, 2018
Closed
@TAGC
Copy link
Copy Markdown

TAGC commented May 19, 2018

I was hoping to capture a screenshot of a specific node in Electron dev tools, but couldn't find the command. Turns out this feature was introduced in Chromium 62 and Electron is on 61 (so close!). It's not an urgent thing I need to do, but I'm curious if there's any ETA yet on the release with 62.

@sethlu sethlu mentioned this pull request Jul 17, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deepak1556 deepak1556 deepak1556 left review comments

+2 more reviewers

@ibash ibash ibash approved these changes

@zcbenz zcbenz zcbenz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@codebytere @alexeykuzmin @DominikLevitsky @zcbenz @mividtim @jkleinsc @MarshallOfSound @TAGC @ibash @deepak1556