feat: add 'ensureWebviewsHavePages' cap #553
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks @jlipps for looking into this. |
lib/webview-helpers.js
Outdated
| if (ensureWebviewsHavePages) { | ||
| logger.info('Retrieved potential webviews; will filter out ones with no active pages'); | ||
| webviewProcs = await asyncfilter(webviewProcs, async (wp) => { | ||
| return await webviewHasPages(adb, wp, webviewDevtoolsPort); |
There was a problem hiding this comment.
return is redundant
There was a problem hiding this comment.
i don't think so, doesn't the filter method need to return true or false in order to do its job?
There was a problem hiding this comment.
I mean
await asyncfilter(webviewProcs, async (wp) => await webviewHasPages(adb, wp, webviewDevtoolsPort))
should work just fine
|
ok @mykola-mokhnach i endured the tedium of updating all the function comments to proper docstrings :-) and found an excuse to use |
mykola-mokhnach
approved these changes
Jul 12, 2019
|
it would be useful to have the new cap documented after the PR is merged |
|
published in 4.18.0. will put in docs PR soon. |
jlipps
added a commit
to jlipps/appium
that referenced
this pull request
Jul 16, 2019
jlipps
added a commit
to appium/appium
that referenced
this pull request
Aug 12, 2019
jlipps
added a commit
to appium/appium
that referenced
this pull request
Aug 26, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes an odd situation which I have observed with Chrome-backed webviews on Android. Currently, Appium's code assumes that if it can find a webview-type process in
/proc/net/unix, then a webview exists for that process.I've noticed that this assumption is not always true. Sometimes, the process exists, and a Chrome remote debugger is even running for the process, but there are no active webview pages. In this case, Chromedriver will always fail to attach to the webview, resulting in an error like "unable to discover open pages".
This PR adds a new capability
ensureWebviewsHavePageswhich, if true, attempts to verify that a webview actually has active pages before adding it to the list of contexts. This serves to prevent users from trying to connect to a webview which is guaranteed to fail, and which will result in a cryptic error message.I've added this behind a capability because the only way I can see to achieve this is to send a request to one of the Chrome devtools HTTP endpoints. This involves running an adb command to forward a local port to the remote debugger port on the device, so we can make the necessary HTTP request, and then cleaning up the forwarded port afterwards. Thus it potentially adds overhead of time, and I'm also not certain that all webviews on all supported versions of Android support the Chrome devtools HTTP server. Once this feature is well-tested and proved useful, we could make this behavior opt-out rather than opt-in.
Because a local port is required for this feature, I've also added another capability,
webviewDevtoolsPort, so that clients can specify a port to avoid port conflicts between sessions.