feat: upgrade to Chromium 73.0.3683.27 #16494

deepak1556 · 2019-01-22T20:46:01Z

Description of Change

Refs https://github.com/electron/electron/projects/17

BREAKING CHANGE

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are changed or added
relevant documentation is changed or added
PR title follows semantic commit guidelines
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: upgrade to Chromium 73.0.3683.27

deepak1556 · 2019-02-08T16:35:46Z

Will rebase once beta branch is cut in upstream, its real close. Lets merge once we have updated to beta release.

deepak1556 · 2019-02-11T04:46:51Z

@MarshallOfSound the nightly-mac-release-test chromedriver workflow failed with

/bin/sh: /Users/.../external_binaries/sccache: cannot execute binary file

looks like its missing step-fix-sync-on-mac step ?

deepak1556 · 2019-02-12T16:46:30Z

Ignore lint errors, they should be fixed by #16906

nornagon · 2019-02-14T00:32:36Z

atom/browser/media/media_capture_devices_dispatcher.cc

 MediaCaptureDevicesDispatcher::GetAudioCaptureDevices() {
  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
  if (is_device_enumeration_disabled_)
-    return EmptyDevices();
+    return test_audio_devices_;


What's the intent behind returning an internal list instead of making a new empty one? This seems like a potential issue if this somehow became non-empty

This is already behind a noop code path because of DisableDeviceEnumerationForTesting. I am inclined to remove this condition all together, the goal of rewriting this was to see if the upstream MediaCaptureDevicesDispatcher class can be reused with less patch.

nornagon · 2019-02-14T01:07:03Z

atom/browser/media/media_stream_devices_controller.cc

        // Get the default devices for the request.
        MediaCaptureDevicesDispatcher::GetInstance()->GetDefaultDevices(
            microphone_requested_, webcam_requested_, &devices);
        break;
+      }
+      case blink::MEDIA_DEVICE_UPDATE: {
+        NOTREACHED();


should we do something else with this event?

It was added in https://chromium-review.googlesource.com/c/chromium/src/+/1354442. The feature seems specifically designed for chrome's screen sharing UI, can consider it for improvement in our desktop capture model once the issue goes public (the chromium issue has restricted view) and provides a bit more context.

nornagon · 2019-02-14T01:11:53Z

atom/common/application_info.cc

+#include "chrome/common/chrome_version.h"
+#include "content/public/common/user_agent.h"
+
+std::string GetUserAgent() {


this should perhaps go in the atom namespace? otherwise, it should have a comment about why it isn't

nornagon · 2019-02-14T01:14:54Z

atom/renderer/api/atom_api_web_frame.cc

-                 &SetIsolatedWorldContentSecurityPolicy);
-  dict.SetMethod("setIsolatedWorldHumanReadableName",
-                 &SetIsolatedWorldHumanReadableName);
+  dict.SetMethod("setIsolatedWorldInfo", &SetIsolatedWorldInfo);


Did we update the docs for this? We should probably continue to support the old API as deprecated for 5-0-x

@nornagon There is a separate PR to deprecate the current API in 5-0-x

nornagon · 2019-02-14T01:26:31Z

patches/common/chromium/can_create_window.patch

-+    body->set_identifier(params->body->identifier);
-+    body->set_contains_sensitive_info(params->body->contains_sensitive_info);
-+  }
-+


can we really get rid of all this stuff? that's great!

network::ResourceRequestBody got typemapped in https://chromium-review.googlesource.com/c/chromium/src/+/1395977 , which helped us get rid off these manual conversions. Side note: mojom typemaps are really cool feature :)

nornagon · 2019-02-14T01:33:39Z

atom/browser/ui/message_box_mac.mm

@@ -157,10 +157,13 @@ int ShowMessageBox(NativeWindow* parent_window,
          callEndModal:true];

  NSWindow* window = parent_window->GetNativeWindow().GetNativeNSWindow();
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
  [alert beginSheetModalForWindow:window
                    modalDelegate:delegate
                   didEndSelector:@selector(alertDidEnd:returnCode:contextInfo:)
                      contextInfo:nil];


can we instead switch to using beginSheetModalForWindow:completionHandler:? here and below

Yup that seems like the alternative, can we change this as a follow up ? There are cards for this in the project board.

nornagon · 2019-02-14T01:34:18Z

build/args/all.gn

+
+# FIXME(deepak1556): workaround for https://crbug.com/924225
+# remove this when clang roll 352138 lands.
+enable_precompiled_headers = false


can we remove this now?

The change got landed in 74 https://chromium-review.googlesource.com/c/chromium/src/+/1436036

https://chromium-review.googlesource.com/c/1392181

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

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

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

…el event https://chromium-review.googlesource.com/c/chromium/src/+/1401664

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

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

Workaround for https://crbug.com/924225

release-clerk · 2019-02-14T20:28:45Z

Release Notes Persisted

Upgraded to Chromium 73.0.3683.27.

trop · 2019-02-14T20:41:16Z

A maintainer has manually backported this PR to "5-0-x", please check out #16975

trop · 2019-02-15T01:31:18Z

A maintainer has manually backported this PR to "5-0-x", please check out #16975

deepak1556 requested review from a team January 22, 2019 20:46

deepak1556 changed the title ~~[WIP] feat: upgrade to Chromium 73.0.3673.0~~ [WIP] feat: upgrade to Chromium 73.0.3679.0 Jan 24, 2019

deepak1556 force-pushed the chromium-upgrade/73 branch 2 times, most recently from d1e9302 to ad96224 Compare January 27, 2019 16:34

deepak1556 force-pushed the chromium-upgrade/73 branch from 426c517 to 37ce80a Compare February 4, 2019 05:50

deepak1556 changed the title ~~[WIP] feat: upgrade to Chromium 73.0.3679.0~~ [WIP] feat: upgrade to Chromium 73.0.3683.10 Feb 4, 2019

deepak1556 force-pushed the chromium-upgrade/73 branch 2 times, most recently from a1cb707 to c6f2e7b Compare February 5, 2019 23:24

deepak1556 changed the title ~~[WIP] feat: upgrade to Chromium 73.0.3683.10~~ feat: upgrade to Chromium 73.0.3683.10 Feb 5, 2019

deepak1556 force-pushed the chromium-upgrade/73 branch from c834c8a to a8c7a7a Compare February 10, 2019 15:21

deepak1556 changed the title ~~feat: upgrade to Chromium 73.0.3683.10~~ feat: upgrade to Chromium 73.0.3683.27 Feb 10, 2019

deepak1556 force-pushed the chromium-upgrade/73 branch from a8c7a7a to 70e4744 Compare February 11, 2019 20:47

deepak1556 mentioned this pull request Feb 12, 2019

ci: fix binaries for chromedriver build on macOS #16893

Merged

6 tasks

deepak1556 closed this in #16893 Feb 12, 2019

deepak1556 reopened this Feb 12, 2019

deepak1556 force-pushed the chromium-upgrade/73 branch from 70e4744 to 470d2be Compare February 12, 2019 16:10

deepak1556 force-pushed the chromium-upgrade/73 branch from 470d2be to 421afdf Compare February 13, 2019 08:18

nornagon reviewed Feb 14, 2019

View reviewed changes

nornagon and others added 17 commits February 14, 2019 23:59

gfx::Path -> SkPath

34d7e01

https://chromium-review.googlesource.com/c/1392181

Rename BlendTowardOppositeLuma() to BlendTowardContrastingEndpoint().

70bbbb8

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

Delete tab pulsing feature.

3727982

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

ui/base: move clipboard to own folder.

e11589b

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

REVIEW: Have MouseWheelPhaseHandler agree on whether to route its whe…

8a51c9c

…el event https://chromium-review.googlesource.com/c/chromium/src/+/1401664

REVIEW: ui::Compositor: Simplify how ExternalBeginFrameClient is used.

76fde58

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

Remove "layouttest" or "layout test" in //content

66db627

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

chore: make OSR compile on Windows

cece8bc

chore: use c++ manifests instead of json

d7557cb

chore: fix linking errors on Windows

2ccf590

REVIEW: fix possible -Wdeprecated-declarations warnings

d7fea1b

Disable precompiled headers

2556d86

Workaround for https://crbug.com/924225

fix compilation error for windows testing builds

b6f7157

FIXME: disable OSR

1d9170f

Disable swiftshader for arm32

95386ef

chore: roll 73.0.3683.27

b927be9

Run electron tests first

46a49b7

deepak1556 force-pushed the chromium-upgrade/73 branch from da12596 to 46a49b7 Compare February 14, 2019 18:31

MarshallOfSound added target/5-0-x and removed target/5-0-x labels Feb 14, 2019

MarshallOfSound approved these changes Feb 14, 2019

View reviewed changes

MarshallOfSound merged commit 5904544 into master Feb 14, 2019

MarshallOfSound mentioned this pull request Feb 14, 2019

feat: upgrade to Chromium 73.0.3683.27 #16975

Merged

MarshallOfSound added the in-flight/5-0-x label Feb 14, 2019

BinaryMuse added merged/5-0-x and removed in-flight/5-0-x labels Feb 20, 2019

BinaryMuse deleted the chromium-upgrade/73 branch February 20, 2019 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade to Chromium 73.0.3683.27 #16494

feat: upgrade to Chromium 73.0.3683.27 #16494

deepak1556 commented Jan 22, 2019 •

edited by nornagon

deepak1556 commented Feb 8, 2019

deepak1556 commented Feb 11, 2019

deepak1556 commented Feb 12, 2019

nornagon Feb 14, 2019

deepak1556 Feb 14, 2019 •

edited

nornagon Feb 14, 2019

deepak1556 Feb 14, 2019 •

edited

nornagon Feb 14, 2019 •

edited

deepak1556 Feb 14, 2019

nornagon Feb 14, 2019

MarshallOfSound Feb 14, 2019

MarshallOfSound Feb 14, 2019

nornagon Feb 14, 2019

deepak1556 Feb 14, 2019

nornagon Feb 14, 2019

deepak1556 Feb 14, 2019 •

edited

nornagon Feb 14, 2019

deepak1556 Feb 14, 2019

release-clerk bot commented Feb 14, 2019 •

edited by nornagon

trop bot commented Feb 14, 2019

trop bot commented Feb 15, 2019

feat: upgrade to Chromium 73.0.3683.27 #16494

feat: upgrade to Chromium 73.0.3683.27 #16494

Conversation

deepak1556 commented Jan 22, 2019 • edited by nornagon

Description of Change

Checklist

Release Notes

deepak1556 commented Feb 8, 2019

deepak1556 commented Feb 11, 2019

deepak1556 commented Feb 12, 2019

Choose a reason for hiding this comment

deepak1556 Feb 14, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepak1556 Feb 14, 2019 • edited

Choose a reason for hiding this comment

nornagon Feb 14, 2019 • edited

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 Feb 14, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

release-clerk bot commented Feb 14, 2019 • edited by nornagon

trop bot commented Feb 14, 2019

trop bot commented Feb 15, 2019

deepak1556 commented Jan 22, 2019 •

edited by nornagon

deepak1556 Feb 14, 2019 •

edited

deepak1556 Feb 14, 2019 •

edited

nornagon Feb 14, 2019 •

edited

deepak1556 Feb 14, 2019 •

edited

release-clerk bot commented Feb 14, 2019 •

edited by nornagon