feat: upgrade to Chromium 70.0.3538.110 #15405

MarshallOfSound · 2018-10-26T05:34:10Z

Description of Change

Upgrade to Chromium 70.0.3538.110
BREAKING CHANGE

Board https://github.com/orgs/electron/projects/25

Build Checklist

Tests Checklist

Release Notes

Notes: Upgrade to Chromium 70.0.3538.79

nornagon · 2018-11-16T18:18:33Z

BUILD.gn

@@ -398,6 +398,7 @@ static_library("electron_lib") {
  }
  if (is_win) {
    libs += [ "dwmapi.lib" ]
+    cflags = [ "-Wno-microsoft-include" ]


If this is a problem with node's headers, then this flag should go in the node GN files.

@nornagon I tried that, it didn't help. The issue comes from Electron including a node header which then causes this error rather than the node build itself.

That's what public_configs is for in GN.

atom/browser/atom_browser_context.cc

nornagon · 2018-11-16T18:28:52Z

spec/api-session-spec.js

+      // FIXME(deepak1556): This is just a hack to force
+      // creation of request context which in turn initializes
+      // the network context, can be removed with network
+      // service enabled.


This seems concerning, and I don't see a card for it on https://github.com/orgs/electron/projects/25. Is this something that users will have to do to make proxies work correctly?

Have created https://github.com/orgs/electron/projects/25#card-15285452 to track it, this only affects when users create a session and doesn't access any network apis, instead directly use the proxy apis which requires a network context initialized. Its a given disadvantage from gluing legacy network code with new network service apis, will fix this after merge.

atom/browser/net/resolve_proxy_helper.cc

atom/browser/node_debugger.cc

nornagon · 2018-11-16T19:11:39Z

atom/common/native_mate_converters/network_converter.cc

-      (*out)->AppendBytes(bytes->GetBlob().data(), bytes->GetBlob().size());
+      (*out)->AppendBytes(
+          reinterpret_cast<const char*>(bytes->GetBlob().data()),
+          base::checked_cast<int>(bytes->GetBlob().size()));


Could this potentially be

(*out)->AppendBytes(bytes->GetBlob());

bytes->GetBlob() is of type BlobStorage which is defined as std::vector<uint8_t> here: https://chromium.googlesource.com/chromium/src/+/70.0.3538.109/base/values.h#83, and AppendBytes takes a std::vector<char>...

nornagon · 2018-11-16T19:48:26Z

atom/renderer/atom_renderer_client.cc

@@ -221,8 +222,11 @@ void AtomRendererClient::SetupMainWorldOverrides(
  dict.Set(options::kNativeWindowOpen,
           command_line->HasSwitch(switches::kNativeWindowOpen));

-  v8::Local<v8::Value> args[] = {binding};
-  ignore_result(func->Call(context, v8::Null(isolate), 1, args));
+  if (result->IsFunction()) {


In what situation would result not be a function? Perhaps we should CHECK here?

nornagon · 2018-11-16T19:48:42Z

atom/renderer/atom_sandboxed_renderer_client.cc

+  if (maybe_script.ToLocal(&script))
+    result = script->Run(context).ToLocalChecked();
+
+  if (result->IsFunction()) {


CHECK might be more appropriate than if here

nornagon · 2018-11-16T19:50:38Z

atom/renderer/atom_sandboxed_renderer_client.cc

+  v8::Local<v8::Script> script;
+  v8::Local<v8::Value> result;
+  if (maybe_script.ToLocal(&script))
+    result = script->Run(context).ToLocalChecked();


Perhaps this logic could be refactored to reuse the logic in CreatePreloadScript?

deepak1556 · 2018-11-29T18:27:52Z

@nornagon have rebased and addressed your comments. Can you review again, thanks!

nornagon · 2018-11-29T19:27:33Z

atom/renderer/renderer_client_base.cc

@@ -278,4 +277,14 @@ v8::Local<v8::Context> RendererClientBase::GetContext(
    return frame->MainWorldScriptContext();
 }

+v8::Local<v8::Value> RendererClientBase::RunScript(


Suggested change

v8::Local<v8::Value> RendererClientBase::RunScript(

// static

v8::Local<v8::Value> RendererClientBase::RunScript(

nornagon · 2018-11-29T19:30:21Z

refs: https://chromium-review.googlesource.com/c/chromium/src/+/1234372

* nodejs/node#22251 * nodejs/node#20137

It is enabled by default now.

release-clerk · 2018-12-03T16:26:53Z

Release Notes Persisted

Upgrade to Chromium 70.0.3538.79

MarshallOfSound requested review from a team October 26, 2018 05:34

MarshallOfSound force-pushed the chromium-upgrade/70 branch from 5591391 to 0510774 Compare October 26, 2018 09:50

codebytere force-pushed the chromium-upgrade/70 branch 2 times, most recently from 36b7c62 to faa465b Compare October 29, 2018 01:01

deepak1556 mentioned this pull request Oct 30, 2018

webview cookies are not persisted #15365

Closed

deepak1556 force-pushed the chromium-upgrade/70 branch 3 times, most recently from 3bcb7c1 to d123726 Compare November 14, 2018 16:05

deepak1556 changed the title ~~feat: upgrade to Chromium 70.0.3538.79~~ feat: upgrade to Chromium 70.0.3538.102 Nov 14, 2018

nornagon reviewed Nov 16, 2018

View reviewed changes

atom/browser/atom_browser_context.cc Show resolved Hide resolved

nornagon reviewed Nov 16, 2018

View reviewed changes

atom/browser/net/resolve_proxy_helper.cc Show resolved Hide resolved

nornagon reviewed Nov 16, 2018

View reviewed changes

atom/browser/node_debugger.cc Outdated Show resolved Hide resolved

nornagon reviewed Nov 16, 2018

View reviewed changes

deepak1556 force-pushed the chromium-upgrade/70 branch from d123726 to ad6799d Compare November 29, 2018 03:23

deepak1556 changed the title ~~feat: upgrade to Chromium 70.0.3538.102~~ feat: upgrade to Chromium 70.0.3538.110 Nov 29, 2018

deepak1556 force-pushed the chromium-upgrade/70 branch from ad6799d to 7f1c753 Compare November 29, 2018 17:24

deepak1556 requested review from alexeykuzmin, ckerr and zcbenz November 29, 2018 18:37

nornagon reviewed Nov 29, 2018

View reviewed changes

nornagon approved these changes Nov 29, 2018

View reviewed changes

MarshallOfSound and others added 26 commits December 3, 2018 20:07

chore: update CreateSkBitmapFromHICON usage for new API (no pointer)

9d72bd4

refs: https://chromium-review.googlesource.com/c/chromium/src/+/1234372

fix: disable kSpareRendererForSitePerProcess feature

28cff46

fix: use proxy lookup api of network context to resolve proxies

7a68d63

spec: force create network context for proxy tests

d97cf8c

spec: update inspector-binding.js

c491fa5

* nodejs/node#22251 * nodejs/node#20137

FIXME: disable some specs

00294a5

fix: update and re-enable OSR

8727636

fix: reimplement FrameSubscriber with mojo, re-enable tests

6deb7af

chore: rm fix_zoom_display.patch

2d68d60

chore: update customizable_app_indicator_id_prefix.patch

2ebea55

chore: update printing.patch

4eb0d49

chore: roll DEPS 70.0.3538.102

c6313b3

fix: remove explicitly enabled SharedArrayBuffer feature flag

5017db8

It is enabled by default now.

chore: update frame_host_manager.patch

65b7924

chore: update fix_trackpad_scrolling.patch

f841855

chore: fix v8 deprecation warnings

e1336d8

chore: use base::JoinString to concatenate vector

06a501d

chore: move node specific build config out of //electron

3dfc009

chore: roll DEPS 70.0.3538.110

e734536

chore: ensure_cookie_store.patch is not needed for Ch70

975f85e

chore: fix_font_thickness.patch is not needed for Ch70

fe943f5

chore: pick_b0af309_from_upstream_v8.patch is not needed for Ch70

f00b0a3

chore: bit_hash_seed_commits.patch is not needed for Ch70

e842482

chore: backport_958b761_from_upstream_v8.patch is not needed for Ch70

a7ee737

chore: update libgtkui_export.patch

982649b

FIXME: disable v8 embedded builtins feature for arm flavor

8ec3bc0

deepak1556 force-pushed the chromium-upgrade/70 branch from 2a28542 to 8ec3bc0 Compare December 3, 2018 14:38

MarshallOfSound merged commit 3ddd917 into master Dec 3, 2018

MarshallOfSound deleted the chromium-upgrade/70 branch December 3, 2018 16:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade to Chromium 70.0.3538.110 #15405

feat: upgrade to Chromium 70.0.3538.110 #15405

MarshallOfSound commented Oct 26, 2018 •

edited

nornagon Nov 16, 2018

MarshallOfSound Nov 17, 2018

nornagon Nov 19, 2018

nornagon Nov 16, 2018

deepak1556 Nov 29, 2018

nornagon Nov 16, 2018

nornagon Nov 16, 2018

nornagon Nov 16, 2018

nornagon Nov 16, 2018

deepak1556 commented Nov 29, 2018

nornagon Nov 29, 2018

nornagon commented Nov 29, 2018

release-clerk bot commented Dec 3, 2018

	v8::Local<v8::Value> RendererClientBase::RunScript(
	// static
	v8::Local<v8::Value> RendererClientBase::RunScript(

feat: upgrade to Chromium 70.0.3538.110 #15405

feat: upgrade to Chromium 70.0.3538.110 #15405

Conversation

MarshallOfSound commented Oct 26, 2018 • edited

Description of Change

Build Checklist

Tests Checklist

Release Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepak1556 commented Nov 29, 2018

Choose a reason for hiding this comment

nornagon commented Nov 29, 2018

release-clerk bot commented Dec 3, 2018

MarshallOfSound commented Oct 26, 2018 •

edited