fix: handle async execute on iOS without awaitPromise (#199)

* fix: handle async execute on iOS without awaitPromise * add e2e test for async execute in frame * address comments
appium · Feb 5, 2020 · f558d4f · f558d4f
1 parent c3a7a2c
commit f558d4f
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 27 deletions.
diff --git a/lib/remote-debugger.js b/lib/remote-debugger.js
@@ -419,59 +419,67 @@ class RemoteDebugger extends events.EventEmitter {
   }
 
   async executeAtomAsync (atom, args, frames) {
+    // helper to send directly to the web inspector
+    const evaluate = async (method, opts) => {
+      return await this.rpcClient.send(method, {
+        ...opts,
+        appIdKey: this.appIdKey,
+        pageIdKey: this.pageIdKey,
+        returnByValue: false,
+      });
+    };
+
     // first create a Promise on the page, saving the resolve/reject functions
     // as properties
     const promiseName = `appiumAsyncExecutePromise${UUID.create().toString().replace(/-/g, '')}`;
-    let script = `var res, rej;` +
-                 `window.${promiseName} = new Promise(function (resolve, reject) {` +
-                 `  res = resolve;` +
-                 `  rej = reject;` +
-                 `});` +
-                 `window.${promiseName}.resolve = res;` +
-                 `window.${promiseName}.reject = rej;` +
-                 `window.${promiseName};`;
-    const obj = await this.rpcClient.send('Runtime.evaluate', {
-      command: script,
-      appIdKey: this.appIdKey,
-      pageIdKey: this.pageIdKey,
-      returnByValue: false,
-    });
+    const script =
+      `var res, rej;
+      window.${promiseName} = new Promise(function (resolve, reject) {
+        res = resolve;
+        rej = reject;
+      });
+      window.${promiseName}.resolve = res;
+      window.${promiseName}.reject = rej;
+      window.${promiseName};`;
+    const obj = await evaluate('Runtime.evaluate', {command: script});
     const promiseObjectId = obj.result.objectId;
 
     // execute the atom, calling back to the resolve function
-    const asyncCallBack = `function (res) {` +
-                          `  window.${promiseName}.resolve(res);` +
-                          `  window.${promiseName}Value = res;` +
-                          `}`;
+    const asyncCallBack =
+      `function (res) {
+        window.${promiseName}.resolve(res);
+        window.${promiseName}Value = res;
+      }`;
     await this.execute(await getScriptForAtom(atom, args, frames, asyncCallBack));
 
     // wait for the promise to be resolved
     let res;
     const subcommandTimeout = 1000; // timeout on individual commands
     try {
-      res = await this.rpcClient.send('Runtime.awaitPromise', {
-        promiseObjectId,
-        appIdKey: this.appIdKey,
-        pageIdKey: this.pageIdKey,
-      });
+      res = await evaluate('Runtime.awaitPromise', {promiseObjectId});
     } catch (err) {
       if (!err.message.includes(`'Runtime.awaitPromise' was not found`)) {
         throw err;
       }
-
       // awaitPromise is not always available, so simulate it with poll
       const retryWait = 100;
       const timeout = (args.length >= 3) ? args[2] : RPC_RESPONSE_TIMEOUT_MS;
       // if the timeout math turns up 0 retries, make sure it happens once
       const retries = parseInt(timeout / retryWait, 10) || 1;
       const timer = new timing.Timer().start();
+      log.debug(`Waiting up to ${timeout}ms for async execute to finish`);
       res = await retryInterval(retries, retryWait, async () => {
         // the atom _will_ return, either because it finished or an error
         // including a timeout error
-        if (await this.executeAtom('execute_script', [`return window.hasOwnProperty('${promiseName}Value');`, [null, null], subcommandTimeout], frames)) {
+        const hasValue = await evaluate('Runtime.evaluate', {
+          command: `window.hasOwnProperty('${promiseName}Value');`,
+        });
+        if (hasValue) {
           // we only put the property on `window` when the callback is called,
           // so if it is there, everything is done
-          return await this.executeAtom('execute_script', [`return window.${promiseName}Value;`, [null, null], subcommandTimeout], frames);
+          return await evaluate('Runtime.evaluate', {
+            command: `window.${promiseName}Value;`,
+          });
         }
         // throw a TimeoutError, or else it needs to be caught and re-thrown
         throw new errors.TimeoutError(`Timed out waiting for asynchronous script ` +

diff --git a/lib/rpc-message-handler.js b/lib/rpc-message-handler.js
@@ -90,7 +90,7 @@ export default class RpcMessageHandler extends EventEmitters {
 
     if (msgId) {
       if (this.listenerCount(msgId)) {
-        if (result?.result?.value) {
+        if (_.has(result?.result, 'value')) {
           result = result.result.value;
         }
         this.emit(msgId, error, result);

diff --git a/test/functional/html/frameset.html b/test/functional/html/frameset.html
@@ -0,0 +1,10 @@
+<html>
+  <head>
+    <title>Remote debugger test page: frames</title>
+  </head>
+<frameset cols="*, *, *">
+    <frame name="first" src="subframe1.html"/>
+    <frame name="second" src="subframe2.html"/>
+    <frame name="third" src="subframe3.html" id="frame3" />
+</frameset>
+</html>
diff --git a/test/functional/html/subframe1.html b/test/functional/html/subframe1.html
@@ -0,0 +1,9 @@
+<html>
+  <head>
+    <title>Remote debugger test page: Sub frame 1</title>
+  </head>
+  <body>
+    <h1>Sub frame 1</h1>
+    <a href="index.html" target="namedwindow">Open a named window</a>
+  </body>
+</html>
diff --git a/test/functional/html/subframe2.html b/test/functional/html/subframe2.html
@@ -0,0 +1,8 @@
+<html>
+  <head>
+    <title>Remote debugger test page: Sub frame 2</title>
+  </head>
+  <body>
+    <h1>Sub frame 2</h1>
+  </body>
+</html>
diff --git a/test/functional/html/subframe3.html b/test/functional/html/subframe3.html
@@ -0,0 +1,8 @@
+<html>
+  <head>
+    <title>Remote debugger test page: Sub frame 3</title>
+  </head>
+  <body>
+    <h1>Sub frame 3</h1>
+  </body>
+</html>
diff --git a/test/functional/safari-e2e-specs.js b/test/functional/safari-e2e-specs.js
@@ -164,6 +164,23 @@ describe('Safari remote debugger', function () {
       await rd.executeAtomAsync('execute_async_script', [script, [], timeout], [])
         .should.eventually.be.rejectedWith(/Timed out waiting for/);
     });
+
+    it('should be able to execute asynchronously in frame', async function () {
+      await connect(rd);
+      const page = _.find(await rd.selectApp(address), (page) => page.title === PAGE_TITLE);
+      const [appIdKey, pageIdKey] = page.id.split('.').map((id) => parseInt(id, 10));
+      await rd.selectPage(appIdKey, pageIdKey);
+
+      // go to the frameset page
+      await rd.navToUrl(`${address}/frameset.html`);
+
+      // get the correct frame
+      const {WINDOW: frame} = await rd.executeAtom('frame_by_id_or_name', ['first'], []);
+
+      const script = `arguments[arguments.length - 1](document.getElementsByTagName('h1')[0].innerHTML);`;
+      await rd.executeAtomAsync('execute_async_script', [script, [], timeout], [frame])
+        .should.eventually.eql('Sub frame 1');
+    });
   });
 
   it(`should be able to call 'selectApp' after already connecting to app`, async function () {