Skip to content

Commit 84126a4

Browse files
bughitbughit
authored
feat: optional typically sync callback for WebFrame#executeJavaScript* (electron#21423)
1 parent 748a917 commit 84126a4

File tree

3 files changed

+198
-24
lines changed

3 files changed

+198
-24
lines changed

‎docs/api/web-frame.md

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,28 +122,45 @@ by its key, which is returned from `webFrame.insertCSS(css)`.
122122

123123
Inserts `text` to the focused element.
124124

125-
### `webFrame.executeJavaScript(code[, userGesture])`
125+
### `webFrame.executeJavaScript(code[, userGesture, callback])`
126126

127127
* `code` String
128128
* `userGesture` Boolean (optional) - Default is `false`.
129+
* `callback` Function (optional) - Called after script has been executed. Unless
130+
the frame is suspended (e.g. showing a modal alert), execution will be
131+
synchronous and the callback will be invoked before the method returns. For
132+
compatibility with an older version of this method, the error parameter is
133+
second.
134+
* `result` Any
135+
* `error` Error
129136

130-
Returns `Promise<any>` - A promise that resolves with the result of the executed code
131-
or is rejected if the result of the code is a rejected promise.
137+
Returns `Promise<any>` - A promise that resolves with the result of the executed
138+
code or is rejected if execution throws or results in a rejected promise.
132139

133140
Evaluates `code` in page.
134141

135142
In the browser window some HTML APIs like `requestFullScreen` can only be
136143
invoked by a gesture from the user. Setting `userGesture` to `true` will remove
137144
this limitation.
138145

139-
### `webFrame.executeJavaScriptInIsolatedWorld(worldId, scripts[, userGesture])`
146+
### `webFrame.executeJavaScriptInIsolatedWorld(worldId, scripts[, userGesture, callback])`
140147

141-
* `worldId` Integer - The ID of the world to run the javascript in, `0` is the default world, `999` is the world used by Electrons `contextIsolation` feature. You can provide any integer here.
148+
* `worldId` Integer - The ID of the world to run the javascript
149+
in, `0` is the default main world (where content runs), `999` is the
150+
world used by Electron's `contextIsolation` feature. Accepts values
151+
in the range 1..536870911.
142152
* `scripts` [WebSource[]](structures/web-source.md)
143153
* `userGesture` Boolean (optional) - Default is `false`.
144-
145-
Returns `Promise<any>` - A promise that resolves with the result of the executed code
146-
or is rejected if the result of the code is a rejected promise.
154+
* `callback` Function (optional) - Called after script has been executed. Unless
155+
the frame is suspended (e.g. showing a modal alert), execution will be
156+
synchronous and the callback will be invoked before the method returns. For
157+
compatibility with an older version of this method, the error parameter is
158+
second.
159+
* `result` Any
160+
* `error` Error
161+
162+
Returns `Promise<any>` - A promise that resolves with the result of the executed
163+
code or is rejected if execution throws or results in a rejected promise.
147164

148165
Works like `executeJavaScript` but evaluates `scripts` in an isolated context.
149166

‎shell/renderer/api/electron_api_web_frame.cc

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "services/service_manager/public/cpp/interface_provider.h"
1616
#include "shell/common/api/api.mojom.h"
1717
#include "shell/common/gin_converters/blink_converter.h"
18+
#include "shell/common/gin_converters/callback_converter.h"
1819
#include "shell/common/gin_helper/dictionary.h"
1920
#include "shell/common/gin_helper/promise.h"
2021
#include "shell/common/node_includes.h"
@@ -110,32 +111,58 @@ class RenderFrameStatus final : public content::RenderFrameObserver {
110111

111112
class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {
112113
public:
114+
// for compatibility with the older version of this, error is after result
115+
using CompletionCallback =
116+
base::OnceCallback<void(const v8::Local<v8::Value>& result,
117+
const v8::Local<v8::Value>& error)>;
118+
113119
explicit ScriptExecutionCallback(
114-
gin_helper::Promise<v8::Local<v8::Value>> promise)
115-
: promise_(std::move(promise)) {}
120+
gin_helper::Promise<v8::Local<v8::Value>> promise,
121+
CompletionCallback callback)
122+
: promise_(std::move(promise)), callback_(std::move(callback)) {}
123+
116124
~ScriptExecutionCallback() override = default;
117125

118126
void Completed(
119127
const blink::WebVector<v8::Local<v8::Value>>& result) override {
128+
v8::Isolate* isolate = v8::Isolate::GetCurrent();
120129
if (!result.empty()) {
121130
if (!result[0].IsEmpty()) {
122131
// Right now only single results per frame is supported.
132+
if (!callback_.is_null())
133+
std::move(callback_).Run(result[0], v8::Undefined(isolate));
123134
promise_.Resolve(result[0]);
124135
} else {
125-
promise_.RejectWithErrorMessage(
136+
const char* error_message =
126137
"Script failed to execute, this normally means an error "
127-
"was thrown. Check the renderer console for the error.");
138+
"was thrown. Check the renderer console for the error.";
139+
if (!callback_.is_null()) {
140+
std::move(callback_).Run(
141+
v8::Undefined(isolate),
142+
v8::Exception::Error(
143+
v8::String::NewFromUtf8(isolate, error_message)
144+
.ToLocalChecked()));
145+
}
146+
promise_.RejectWithErrorMessage(error_message);
128147
}
129148
} else {
130-
promise_.RejectWithErrorMessage(
149+
const char* error_message =
131150
"WebFrame was removed before script could run. This normally means "
132-
"the underlying frame was destroyed");
151+
"the underlying frame was destroyed";
152+
if (!callback_.is_null()) {
153+
std::move(callback_).Run(
154+
v8::Undefined(isolate),
155+
v8::Exception::Error(v8::String::NewFromUtf8(isolate, error_message)
156+
.ToLocalChecked()));
157+
}
158+
promise_.RejectWithErrorMessage(error_message);
133159
}
134160
delete this;
135161
}
136162

137163
private:
138164
gin_helper::Promise<v8::Local<v8::Value>> promise_;
165+
CompletionCallback callback_;
139166

140167
DISALLOW_COPY_AND_ASSIGN(ScriptExecutionCallback);
141168
};
@@ -373,9 +400,14 @@ v8::Local<v8::Promise> ExecuteJavaScript(gin_helper::Arguments* args,
373400
bool has_user_gesture = false;
374401
args->GetNext(&has_user_gesture);
375402

403+
ScriptExecutionCallback::CompletionCallback completion_callback;
404+
args->GetNext(&completion_callback);
405+
376406
GetRenderFrame(window)->GetWebFrame()->RequestExecuteScriptAndReturnValue(
377407
blink::WebScriptSource(blink::WebString::FromUTF16(code)),
378-
has_user_gesture, new ScriptExecutionCallback(std::move(promise)));
408+
has_user_gesture,
409+
new ScriptExecutionCallback(std::move(promise),
410+
std::move(completion_callback)));
379411

380412
return handle;
381413
}
@@ -389,6 +421,16 @@ v8::Local<v8::Promise> ExecuteJavaScriptInIsolatedWorld(
389421
gin_helper::Promise<v8::Local<v8::Value>> promise(isolate);
390422
v8::Local<v8::Promise> handle = promise.GetHandle();
391423

424+
bool has_user_gesture = false;
425+
args->GetNext(&has_user_gesture);
426+
427+
blink::WebLocalFrame::ScriptExecutionType scriptExecutionType =
428+
blink::WebLocalFrame::kSynchronous;
429+
args->GetNext(&scriptExecutionType);
430+
431+
ScriptExecutionCallback::CompletionCallback completion_callback;
432+
args->GetNext(&completion_callback);
433+
392434
std::vector<blink::WebScriptSource> sources;
393435

394436
for (const auto& script : scripts) {
@@ -399,7 +441,15 @@ v8::Local<v8::Promise> ExecuteJavaScriptInIsolatedWorld(
399441
script.Get("startLine", &start_line);
400442

401443
if (!script.Get("code", &code)) {
402-
promise.RejectWithErrorMessage("Invalid 'code'");
444+
const char* error_message = "Invalid 'code'";
445+
if (!completion_callback.is_null()) {
446+
std::move(completion_callback)
447+
.Run(v8::Undefined(isolate),
448+
v8::Exception::Error(
449+
v8::String::NewFromUtf8(isolate, error_message)
450+
.ToLocalChecked()));
451+
}
452+
promise.RejectWithErrorMessage(error_message);
403453
return handle;
404454
}
405455

@@ -408,19 +458,14 @@ v8::Local<v8::Promise> ExecuteJavaScriptInIsolatedWorld(
408458
blink::WebURL(GURL(url)), start_line));
409459
}
410460

411-
bool has_user_gesture = false;
412-
args->GetNext(&has_user_gesture);
413-
414-
blink::WebLocalFrame::ScriptExecutionType scriptExecutionType =
415-
blink::WebLocalFrame::kSynchronous;
416-
args->GetNext(&scriptExecutionType);
417-
418461
// Debugging tip: if you see a crash stack trace beginning from this call,
419462
// then it is very likely that some exception happened when executing the
420463
// "content_script/init.js" script.
421464
GetRenderFrame(window)->GetWebFrame()->RequestExecuteScriptInIsolatedWorld(
422465
world_id, &sources.front(), sources.size(), has_user_gesture,
423-
scriptExecutionType, new ScriptExecutionCallback(std::move(promise)));
466+
scriptExecutionType,
467+
new ScriptExecutionCallback(std::move(promise),
468+
std::move(completion_callback)));
424469

425470
return handle;
426471
}

‎spec/api-web-frame-spec.js

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,116 @@ describe('webFrame module', function () {
2525
it('findFrameByRoutingId() does not crash when not found', () => {
2626
expect(webFrame.findFrameByRoutingId(-1)).to.be.null()
2727
})
28+
29+
describe('executeJavaScript', () => {
30+
let childFrameElement, childFrame
31+
32+
before(() => {
33+
childFrameElement = document.createElement('iframe')
34+
document.body.appendChild(childFrameElement)
35+
childFrame = webFrame.firstChild
36+
})
37+
38+
after(() => {
39+
childFrameElement.remove()
40+
})
41+
42+
it('executeJavaScript() yields results via a promise and a sync callback', done => {
43+
let callbackResult, callbackError
44+
45+
childFrame
46+
.executeJavaScript('1 + 1', (result, error) => {
47+
callbackResult = result
48+
callbackError = error
49+
})
50+
.then(
51+
promiseResult => {
52+
expect(promiseResult).to.equal(2)
53+
done()
54+
},
55+
promiseError => {
56+
done(promiseError)
57+
}
58+
)
59+
60+
expect(callbackResult).to.equal(2)
61+
expect(callbackError).to.be.undefined()
62+
})
63+
64+
it('executeJavaScriptInIsolatedWorld() yields results via a promise and a sync callback', done => {
65+
let callbackResult, callbackError
66+
67+
childFrame
68+
.executeJavaScriptInIsolatedWorld(999, [{ code: '1 + 1' }], (result, error) => {
69+
callbackResult = result
70+
callbackError = error
71+
})
72+
.then(
73+
promiseResult => {
74+
expect(promiseResult).to.equal(2)
75+
done()
76+
},
77+
promiseError => {
78+
done(promiseError)
79+
}
80+
)
81+
82+
expect(callbackResult).to.equal(2)
83+
expect(callbackError).to.be.undefined()
84+
})
85+
86+
it('executeJavaScript() yields errors via a promise and a sync callback', done => {
87+
let callbackResult, callbackError
88+
89+
childFrame
90+
.executeJavaScript('thisShouldProduceAnError()', (result, error) => {
91+
callbackResult = result
92+
callbackError = error
93+
})
94+
.then(
95+
promiseResult => {
96+
done(new Error('error is expected'))
97+
},
98+
promiseError => {
99+
expect(promiseError).to.be.an('error')
100+
done()
101+
}
102+
)
103+
104+
expect(callbackResult).to.be.undefined()
105+
expect(callbackError).to.be.an('error')
106+
})
107+
108+
// executeJavaScriptInIsolatedWorld is failing to detect exec errors and is neither
109+
// rejecting nor passing the error to the callback. This predates the reintroduction
110+
// of the callback so will not be fixed as part of the callback PR
111+
// if/when this is fixed the test can be uncommented.
112+
//
113+
// it('executeJavaScriptInIsolatedWorld() yields errors via a promise and a sync callback', done => {
114+
// let callbackResult, callbackError
115+
//
116+
// childFrame
117+
// .executeJavaScriptInIsolatedWorld(999, [{ code: 'thisShouldProduceAnError()' }], (result, error) => {
118+
// callbackResult = result
119+
// callbackError = error
120+
// })
121+
// .then(
122+
// promiseResult => {
123+
// done(new Error('error is expected'))
124+
// },
125+
// promiseError => {
126+
// expect(promiseError).to.be.an('error')
127+
// done()
128+
// }
129+
// )
130+
//
131+
// expect(callbackResult).to.be.undefined()
132+
// expect(callbackError).to.be.an('error')
133+
// })
134+
135+
it('executeJavaScript(InIsolatedWorld) can be used without a callback', async () => {
136+
expect(await webFrame.executeJavaScript('1 + 1')).to.equal(2)
137+
expect(await webFrame.executeJavaScriptInIsolatedWorld(999, [{ code: '1 + 1' }])).to.equal(2)
138+
})
139+
})
28140
})

0 commit comments

Comments
 (0)