CLI: close tab when we're done with it. #1543
Conversation
| resolve(JSON.parse(data)); | ||
| return; | ||
| } catch (e) { | ||
| if (data === 'Target is closing') { |
There was a problem hiding this comment.
The chromium code that handles this says
SendJson(connection_id, net::HTTP_OK, NULL, "Target is closing");But unfortunately, SendJson doesn't send JSON in this case. It sends this raw string.
Thus, the try/catch on this guy.
There was a problem hiding this comment.
Will you add some variant of this as a comment in the code? At least for the string being checked against
| this._ws.close(); | ||
| this._ws = null; | ||
| return Promise.resolve(); | ||
| return this._runJsonCommand(`close/${this._pageId}`).then(_ => { |
There was a problem hiding this comment.
Will this close the browser too if it's the last tab left?
There was a problem hiding this comment.
It will not. It'll close the "window" but not the application.
| return this._runJsonCommand(`close/${this._pageId}`).then(_ => { | ||
| this._ws.removeAllListeners(); | ||
| this._ws.close(); | ||
| this._ws = null; |
There was a problem hiding this comment.
I don't know when we'd ever reuse a cri connection instance, but since we null out _ws, null out _pageId as well?
| } catch (e) { | ||
| if (data === 'Target is closing') { | ||
| return resolve({message: data}); | ||
| } |
There was a problem hiding this comment.
Since you have the try/catch now, want to add a return reject(e) here, even though it wasn't caught in the promise chain before?
|
PTAL |
|
looks like some of the gather-runner tests failed because |
Fixes #1419