diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index f19ac34a5d64..e6959c887111 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1181,15 +1181,14 @@ void WebContents::SendInputEvent(v8::Isolate* isolate, isolate, "Invalid event object"))); } -void WebContents::BeginFrameSubscription( - mate::Arguments* args) { - FrameSubscriber::FrameCaptureCallback callback; +void WebContents::BeginFrameSubscription(mate::Arguments* args) { bool only_dirty = false; + FrameSubscriber::FrameCaptureCallback callback; + args->GetNext(&only_dirty); if (!args->GetNext(&callback)) { - args->GetNext(&only_dirty); - if (!args->GetNext(&callback)) - args->ThrowTypeError("'callback' must be defined"); + args->ThrowError(); + return; } const auto view = web_contents()->GetRenderWidgetHostView(); diff --git a/atom/browser/api/frame_subscriber.cc b/atom/browser/api/frame_subscriber.cc index 91b6765db189..d6d2cf759e14 100644 --- a/atom/browser/api/frame_subscriber.cc +++ b/atom/browser/api/frame_subscriber.cc @@ -5,8 +5,8 @@ #include "atom/browser/api/frame_subscriber.h" #include "base/bind.h" -#include "atom/common/node_includes.h" #include "atom/common/native_mate_converters/gfx_converter.h" +#include "atom/common/node_includes.h" #include "content/public/browser/render_widget_host.h" namespace atom { @@ -17,8 +17,11 @@ FrameSubscriber::FrameSubscriber(v8::Isolate* isolate, content::RenderWidgetHostView* view, const FrameCaptureCallback& callback, bool only_dirty) - : isolate_(isolate), view_(view), callback_(callback), - only_dirty_(only_dirty), weak_factory_(this) { + : isolate_(isolate), + view_(view), + callback_(callback), + only_dirty_(only_dirty), + weak_factory_(this) { } bool FrameSubscriber::ShouldCaptureFrame( @@ -48,8 +51,9 @@ bool FrameSubscriber::ShouldCaptureFrame( } void FrameSubscriber::OnFrameDelivered(const FrameCaptureCallback& callback, - const gfx::Rect& damage_rect, const SkBitmap& bitmap, - content::ReadbackResponse response) { + const gfx::Rect& damage_rect, + const SkBitmap& bitmap, + content::ReadbackResponse response) { if (response != content::ReadbackResponse::READBACK_SUCCESS) return; @@ -67,7 +71,7 @@ void FrameSubscriber::OnFrameDelivered(const FrameCaptureCallback& callback, rgb_arr_size); v8::Local damage = - mate::Converter::ToV8(isolate_, damage_rect); + mate::Converter::ToV8(isolate_, damage_rect); callback_.Run(buffer.ToLocalChecked(), damage); } diff --git a/atom/browser/api/frame_subscriber.h b/atom/browser/api/frame_subscriber.h index b6cff3da81db..0da554ac4795 100644 --- a/atom/browser/api/frame_subscriber.h +++ b/atom/browser/api/frame_subscriber.h @@ -21,7 +21,7 @@ namespace api { class FrameSubscriber : public content::RenderWidgetHostViewFrameSubscriber { public: using FrameCaptureCallback = - base::Callback, v8::Local)>; + base::Callback, v8::Local)>; FrameSubscriber(v8::Isolate* isolate, content::RenderWidgetHostView* view, @@ -35,8 +35,9 @@ class FrameSubscriber : public content::RenderWidgetHostViewFrameSubscriber { private: void OnFrameDelivered(const FrameCaptureCallback& callback, - const gfx::Rect& damage_rect, const SkBitmap& bitmap, - content::ReadbackResponse response); + const gfx::Rect& damage_rect, + const SkBitmap& bitmap, + content::ReadbackResponse response); v8::Isolate* isolate_; content::RenderWidgetHostView* view_; diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index c25b640f91e5..913504cd3745 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -921,7 +921,7 @@ For the `mouseWheel` event, the `event` object also have following properties: ### `webContents.beginFrameSubscription([onlyDirty ,]callback)` -* `onlyDirty` Boolean +* `onlyDirty` Boolean (optional) - Defaults to `false` * `callback` Function Begin subscribing for presentation events and captured frames, the `callback` diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index b5f55905589a..25b852946ff4 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -44,7 +44,10 @@ describe('browser-window module', function () { w = new BrowserWindow({ show: false, width: 400, - height: 400 + height: 400, + webPreferences: { + backgroundThrottling: false + } }) }) @@ -603,12 +606,20 @@ describe('browser-window module', function () { }) describe('beginFrameSubscription method', function () { - it('subscribes to frame updates', function (done) { - this.timeout(20000) + // This test is too slow, only test it on CI. + if (!isCI) return + this.timeout(20000) + + it('subscribes to frame updates', function (done) { + let called = false w.loadURL('file://' + fixtures + '/api/frame-subscriber.html') w.webContents.on('dom-ready', function () { w.webContents.beginFrameSubscription(function (data) { + // This callback might be called twice. + if (called) return + called = true + assert.notEqual(data.length, 0) w.webContents.endFrameSubscription() done() @@ -617,11 +628,14 @@ describe('browser-window module', function () { }) it('subscribes to frame updates (only dirty rectangle)', function (done) { - this.timeout(20000) - + let called = false w.loadURL('file://' + fixtures + '/api/frame-subscriber.html') w.webContents.on('dom-ready', function () { w.webContents.beginFrameSubscription(true, function (data) { + // This callback might be called twice. + if (called) return + called = true + assert.notEqual(data.length, 0) w.webContents.endFrameSubscription() done() @@ -630,10 +644,8 @@ describe('browser-window module', function () { }) it('throws error when subscriber is not well defined', function (done) { - this.timeout(20000) - w.loadURL('file://' + fixtures + '/api/frame-subscriber.html') - try{ + try { w.webContents.beginFrameSubscription(true, true) } catch(e) { done()