Skip to content

/electron

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

fix: handle remote-debugging-port=0 correctly #17800

Merged
merged 1 commit into from Apr 16, 2019
+24 −9

Conversation

Reviewers

@jkleinsc jkleinsc

@zcbenz zcbenz

Assignees
No one assigned
Labels
merged/5-0-x new-pr 🌱
Projects
None yet
Milestone
No milestone
3 participants
@MarshallOfSound @jkleinsc @zcbenz
@MarshallOfSound
Copy link
Member

commented Apr 15, 2019 •
edited

By default the Chromedriver will send remote-debugging-port=0 to let the browser choose a free port to listen on. The chosen port is written to a known file in the user data dir that is passed to the app through the CLI.

This PR does two things.

  1. Correctly passes the USER_DATA_DIR to the remote debugging server so it knows where to write the file
  2. Adds support for --user-data-dir as we did not support that CLI argument and Chromedriver relies on being able to tell the "browser" where to write this file.

Fixes #17354

Notes: Fixed issue where chromedriver would not connect correctly if you didn't provide a custom remote-debugging-port

@electron-cation electron-cation bot added the new-pr 🌱 label Apr 15, 2019

@MarshallOfSound MarshallOfSound added the target/5-0-x label Apr 15, 2019

@MarshallOfSound MarshallOfSound force-pushed the fix-chromedriver branch from 906c21e to 3092911 Apr 15, 2019

@MarshallOfSound
fix: handle remote-debugging-port=0 correctly 
By default the Chromedriver will send remote-debugging-port=0 to let the
browser choose a free port to listen on.  The chosen port is written to
a known file in the user data dir that is passed to the app through the
CLI.

This PR does two things.

1. Correctly passes the USER_DATA_DIR to the remote debugging server so
it knows where to write the file
2. Adds support for --user-data-dir as we did not support that CLI
argument and Chromedriver relies on being able to tell the "browser"
where to write this file.

Fixes #17354
Loading status checks…
30e28ef

@MarshallOfSound MarshallOfSound force-pushed the fix-chromedriver branch from 3092911 to 30e28ef Apr 15, 2019

@zcbenz

zcbenz approved these changes Apr 16, 2019
View changes

@jkleinsc

jkleinsc approved these changes Apr 16, 2019
View changes

@MarshallOfSound MarshallOfSound merged commit b7b9efa into master Apr 16, 2019
8 checks passed

8 checks passed

Semantic Pull Request ready to be squashed
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-debug AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Sign in to view
Copy link

commented Apr 16, 2019

Release Notes Persisted

Fixed issue where chromedriver would not connect correctly if you didn't provide a custom remote-debugging-port

@MarshallOfSound MarshallOfSound deleted the fix-chromedriver branch Apr 16, 2019

@trop

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Apr 16, 2019

I was unable to backport this PR to "5-0-x" cleanly;
you will need to perform this backport manually.

@trop trop bot added needs-manual-bp/5-0-x and removed target/5-0-x labels Apr 16, 2019

MarshallOfSound added a commit that referenced this pull request Apr 16, 2019

@MarshallOfSound
fix: handle remote-debugging-port=0 correctly (#17800) 
By default the Chromedriver will send remote-debugging-port=0 to let the
browser choose a free port to listen on.  The chosen port is written to
a known file in the user data dir that is passed to the app through the
CLI.

This PR does two things.

1. Correctly passes the USER_DATA_DIR to the remote debugging server so
it knows where to write the file
2. Adds support for --user-data-dir as we did not support that CLI
argument and Chromedriver relies on being able to tell the "browser"
where to write this file.

Fixes #17354
Loading status checks…
db6db91

@MarshallOfSound MarshallOfSound referenced this pull request Apr 16, 2019

Merged

fix: handle remote-debugging-port=0 correctly #17817

@trop

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Apr 16, 2019

A maintainer has manually backported this PR to "5-0-x", please check out #17817
1 similar comment
@trop

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Apr 16, 2019

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

MarshallOfSound added a commit that referenced this pull request Apr 16, 2019

@MarshallOfSound
fix: handle remote-debugging-port=0 correctly (#17800) (#17817) 
By default the Chromedriver will send remote-debugging-port=0 to let the
browser choose a free port to listen on.  The chosen port is written to
a known file in the user data dir that is passed to the app through the
CLI.

This PR does two things.

1. Correctly passes the USER_DATA_DIR to the remote debugging server so
it knows where to write the file
2. Adds support for --user-data-dir as we did not support that CLI
argument and Chromedriver relies on being able to tell the "browser"
where to write this file.

Fixes #17354
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
c326de8

@MarshallOfSound MarshallOfSound added merged/5-0-x and removed needs-manual-bp/5-0-x labels Apr 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.