Skip to content

Conversation

deiwin
Copy link
Contributor

@deiwin deiwin commented Sep 25, 2017

The Dockerfile says Chrome beta is supported:

can specify versions by CHROME_VERSION;
e.g. google-chrome-stable=53.0.2785.101-1
google-chrome-beta=53.0.2785.92-1

But when actually building an image with e.g.
--build-arg CHROME_VERSION=google-chrome-beta=62.0.3202.29-1, then a couple
of problems appear. Firstly, the version field in /opt/selenium/config.json
will not be populated, because /opt/google/chrome/chrome does not exist.
Secondly and even more importantly, the node simply won't work. I assume it's
because chrome_launcher.sh tries to execute the nonexistent
/opt/google/chrome/chrome, but I haven't confirmed for sure.

This change fixes both of those issues by actually wrapping the existing
executable, instead of replacing it. I have locally tested it by with latest
versions (not specifying any build arguments) and with the latest Chrome beta
version. Both work.

As can be seen from current Chromium source, the wrapping code that was
used in chrome_launcher.sh has changed considerably. This change avoids
duplicating that code here and so makes it impossible for this to get out of
date. In addition, this will work with different Chrome versions which might
have different code in the wrapper.

@deiwin
Copy link
Contributor Author

deiwin commented Sep 25, 2017

For anybody else running into this, we're using the following desired capabilities to work around this issue at the moment:

"chrome_options": {
  "args": ["--no-sandbox"],
  "binary": "/usr/bin/google-chrome"
}

@diemol
Copy link
Member

diemol commented Sep 29, 2017

Hi @deiwin,

Can you please update your fork with the latest changes from master? Please only change the Dockerfile.txt files.

The Dockerfile says Chrome beta is supported:
>  can specify versions by CHROME_VERSION;
>  e.g. google-chrome-stable=53.0.2785.101-1
>       google-chrome-beta=53.0.2785.92-1

But when actually building an image with e.g.
`--build-arg CHROME_VERSION=google-chrome-beta=62.0.3202.29-1`, then a couple
of problems appear. Firstly, the version field in `/opt/selenium/config.json`
will not be populated, because `/opt/google/chrome/chrome` does not exist.
Secondly and even more importantly, the node simply won't work. I assume it's
because `chrome_launcher.sh` tries to execute the nonexistent
`/opt/google/chrome/chrome`, but I haven't confirmed for sure.

This change fixes both of those issues by actually wrapping the existing
executable, instead of replacing it. I have locally tested it by with latest
versions (not specifying any build arguments) and with the latest Chrome beta
version. Both work.

As can be seen from current [Chromium source][1], the wrapping code that was
used in chrome_launcher.sh has changed considerably. This change avoids
duplicating that code here and so makes it impossible for this to get out of
date. In addition, this will work with different Chrome versions which might
have different code in the wrapper.

[1]: https://cs.chromium.org/chromium/src/chrome/installer/linux/common/wrapper
@deiwin
Copy link
Contributor Author

deiwin commented Sep 29, 2017

Sure thing! Updated.

Let me know if you have any other requests, @diemol. Thanks!

@diemol
Copy link
Member

diemol commented Oct 4, 2017

I just tried it myself and I bumped into the issue when trying to use Chrome Beta.

I briefly reviewed and it looks good, I will test it and review it more in detail between today and tomorrow, and hopefully we can merge soon.

@ddavison, do you want to take a look?

@diemol
Copy link
Member

diemol commented Oct 5, 2017

Thanks for the PR @deiwin, I used it to build a custom image with Chrome Beta and it works fine. I'll merge it and make a release.

@diemol diemol merged commit 6aec1ce into SeleniumHQ:master Oct 5, 2017
@diemol diemol mentioned this pull request Oct 5, 2017
1 task
@deiwin deiwin deleted the fix_chrome_beta_support branch October 6, 2017 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants