Add support for iOS 12.2 #118
Conversation
| handleMessage (plist) { | ||
| let handlerFor = plist.__selector; | ||
| if (!handlerFor) { | ||
| async handleMessage (plist) { |
There was a problem hiding this comment.
have you verified this method is called with await everywhere now?
| let method = dataKey.method; | ||
| let params; | ||
| if (this.targetBased) { | ||
| if (method === 'Target.targetCreated') { |
There was a problem hiding this comment.
I'd rather move this code into a separate method. The current one is already quite long
| export default class RemoteDebuggerRpcClient { | ||
| constructor (opts = {}) { | ||
| const { | ||
| let { | ||
| platformVersion = {}, |
There was a problem hiding this comment.
we are currently not getting xcode version for real devices, because it might not be installed on the target machine. Do we want to change that?
There was a problem hiding this comment.
We do not want to change that. It limits how iOS real device testing can be done.
But in any case we will have a platformVersion, which is what we depend on (since iOS below 12.2, but still Xcode 10.2, continues to use the old mechanism).
| data.__argument.WIRSocketDataKey.id = this.curMsgId; | ||
| data.__argument.WIRSocketDataKey = | ||
| Buffer.from(JSON.stringify(data.__argument.WIRSocketDataKey)); | ||
| if (cmd.__argument.WIRSocketDataKey.params) { |
There was a problem hiding this comment.
hell it's full of magic
| return command('Page.navigate', {url}, appIdKey, connId, | ||
| senderId, pageIdKey, debuggerType); | ||
| } | ||
| indicateWebView (connId, appIdKey, pageIdKey, opts) { |
| */ | ||
|
|
||
| command (method, params, appIdKey, connId, senderId, pageIdKey, debuggerType) { | ||
| if (debuggerType !== null && debuggerType === DEBUGGER_TYPES.webkit) { |
There was a problem hiding this comment.
debuggerType !== null might be skipped
| command (method, params, appIdKey, connId, senderId, pageIdKey, debuggerType) { | ||
| if (debuggerType !== null && debuggerType === DEBUGGER_TYPES.webkit) { | ||
| return this.commandWebKit(method, params); | ||
| } else { |
| } | ||
|
|
||
| return cmd; | ||
| getRemoteCommand (command, opts) { |
| case 'indicateWebView': | ||
| cmd = this.indicateWebView(connId, appIdKey, pageIdKey, opts); | ||
| break; | ||
| case 'sendJSCommand': |
There was a problem hiding this comment.
cannot we simply call this[command](...) for all the other commands with equal arguments?
|
it might be a good idea to add iOS 12.2 to Travis |
|
The build for this package does not actually depend on Xcode at all. It is entirely through a test server we built. This should be changed, but it outside the scope of this work (mostly because it is a huge task). |
| // pageDict is a promise, so resolve | ||
| this.appDict[appIdKey].pageDict.resolve(pageArray); | ||
| if (this.appDict[appIdKey].pageArray) { | ||
| if (this.appDict[appIdKey].pageArray.resolve) { |
| if (result.result && result.result.value) { | ||
| result = result.result.value; | ||
| } | ||
| this.dataHandlers[msgId](result); |
There was a problem hiding this comment.
These are always regular functions.
| logFullMessage (plist) { | ||
| // Buffers cannot be serialized in a readable way | ||
| const bufferToJSON = Buffer.prototype.toJSON; | ||
| delete Buffer.prototype.toJSON; |
There was a problem hiding this comment.
interesting hack %)
| if (this.appDict[appIdKey].pageArray) { | ||
| if (this.appDict[appIdKey].pageArray.resolve) { | ||
| // pageDict is a pending promise, so resolve | ||
| this.appDict[appIdKey].pageArray.resolve(); |
There was a problem hiding this comment.
resolve is just a function. The promise itself it awaited elsewhere (or has already timed out).
The problem: In iOS 12.2 the remote debugger stopped working, because instead of using the entire protocol, all communication is now wrapped in
Target.sendMessageToTargetcommands andTarget.dispatchMessageFromTargetmessages (which prior to iOS 12.2 were not known at all by Safari).This PR implements, for iOS 12.2, this protocol change.
As I worked on this I also cleaned some stuff up, which muddled the diffs a bit, for which I apologize.
Confirmation that this works through real functional tests (with some necessary changes at the driver level):
This works for simulators only. Beginning work on real device integration.
Re: appium/appium#12239 and appium/appium#12240