From 5118def724d650a95360828c45a572f1d1ee9deb Mon Sep 17 00:00:00 2001 From: Heilig Benedek Date: Sat, 25 Jun 2016 18:23:40 +0200 Subject: [PATCH] damaged -> dirty rename, fixed misc issues, changed signature, updated docs and added tests --- atom/browser/api/atom_api_web_contents.cc | 12 +++--- atom/browser/api/frame_subscriber.cc | 12 +++--- atom/browser/api/frame_subscriber.h | 4 +- docs/api/web-contents.md | 12 +++--- spec/api-browser-window-spec.js | 45 +++++++++++++++++------ spec/fixtures/api/frame-subscriber.html | 11 ++++++ 6 files changed, 65 insertions(+), 31 deletions(-) create mode 100644 spec/fixtures/api/frame-subscriber.html diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 1a2318f404..bfedddce8a 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1197,16 +1197,18 @@ void WebContents::SendInputEvent(v8::Isolate* isolate, void WebContents::BeginFrameSubscription( mate::Arguments* args) { FrameSubscriber::FrameCaptureCallback callback; - if (!args->GetNext(&callback)) - return; + bool only_dirty = false; - bool only_damaged = false; - args->GetNext(&only_damaged); + if (!args->GetNext(&callback)) { + args->GetNext(&only_dirty); + if (!args->GetNext(&callback)) + args->ThrowTypeError("'callback' must be defined"); + } const auto view = web_contents()->GetRenderWidgetHostView(); if (view) { std::unique_ptr frame_subscriber(new FrameSubscriber( - isolate(), view, callback, only_damaged)); + isolate(), view, callback, only_dirty)); view->BeginFrameSubscription(std::move(frame_subscriber)); } } diff --git a/atom/browser/api/frame_subscriber.cc b/atom/browser/api/frame_subscriber.cc index db78e7e936..91b6765db1 100644 --- a/atom/browser/api/frame_subscriber.cc +++ b/atom/browser/api/frame_subscriber.cc @@ -16,13 +16,13 @@ namespace api { FrameSubscriber::FrameSubscriber(v8::Isolate* isolate, content::RenderWidgetHostView* view, const FrameCaptureCallback& callback, - const bool& only_damaged) + bool only_dirty) : isolate_(isolate), view_(view), callback_(callback), - only_damaged_(only_damaged), weak_factory_(this) { + only_dirty_(only_dirty), weak_factory_(this) { } bool FrameSubscriber::ShouldCaptureFrame( - const gfx::Rect& damage_rect, + const gfx::Rect& dirty_rect, base::TimeTicks present_time, scoped_refptr* storage, DeliverFrameCallback* callback) { @@ -30,12 +30,12 @@ bool FrameSubscriber::ShouldCaptureFrame( if (!view_ || !host) return false; - if (damage_rect.width() == 0 || damage_rect.height() == 0) + if (dirty_rect.IsEmpty()) return false; gfx::Rect rect = gfx::Rect(view_->GetVisibleViewportSize()); - if (only_damaged_) - rect = damage_rect; + if (only_dirty_) + rect = dirty_rect; host->CopyFromBackingStore( rect, diff --git a/atom/browser/api/frame_subscriber.h b/atom/browser/api/frame_subscriber.h index 9ab7f1eb9b..b6cff3da81 100644 --- a/atom/browser/api/frame_subscriber.h +++ b/atom/browser/api/frame_subscriber.h @@ -26,7 +26,7 @@ class FrameSubscriber : public content::RenderWidgetHostViewFrameSubscriber { FrameSubscriber(v8::Isolate* isolate, content::RenderWidgetHostView* view, const FrameCaptureCallback& callback, - const bool& only_damaged); + bool only_dirty); bool ShouldCaptureFrame(const gfx::Rect& damage_rect, base::TimeTicks present_time, @@ -41,7 +41,7 @@ class FrameSubscriber : public content::RenderWidgetHostViewFrameSubscriber { v8::Isolate* isolate_; content::RenderWidgetHostView* view_; FrameCaptureCallback callback_; - bool only_damaged_; + bool only_dirty_; base::WeakPtrFactory weak_factory_; diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 3d953a0a27..56263780a0 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -917,13 +917,13 @@ For the `mouseWheel` event, the `event` object also have following properties: * `hasPreciseScrollingDeltas` Boolean * `canScroll` Boolean -### `webContents.beginFrameSubscription(callback, onlyDamaged)` +### `webContents.beginFrameSubscription([onlyDirty ,]callback)` +* `onlyDirty` Boolean * `callback` Function -* `onlyDamaged` Boolean Begin subscribing for presentation events and captured frames, the `callback` -will be called with `callback(frameBuffer, damagedRect)` when there is a +will be called with `callback(frameBuffer, dirtyRect)` when there is a presentation event. The `frameBuffer` is a `Buffer` that contains raw pixel data. On most machines, @@ -932,9 +932,9 @@ representation depends on the endianness of the processor (most modern processors are little-endian, on machines with big-endian processors the data is in 32bit ARGB format). -The `damagedRect` is an object with `x, y, width, height` properties that -describes which part of the page was repainted. If `onlyDamaged` is set to -`true`, `frameBuffer` will only contain the repainted area. `onlyDamaged` +The `dirtyRect` is an object with `x, y, width, height` properties that +describes which part of the page was repainted. If `onlyDirty` is set to +`true`, `frameBuffer` will only contain the repainted area. `onlyDirty` defaults to `false`. ### `webContents.endFrameSubscription()` diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 7310cd6a90..b5f5590558 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -603,21 +603,42 @@ describe('browser-window module', function () { }) describe('beginFrameSubscription method', function () { - this.timeout(20000) + it('subscribes to frame updates', function (done) { + this.timeout(20000) - it('subscribes frame updates', function (done) { - let called = false - w.loadURL('file://' + fixtures + '/api/blank.html') - 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() + w.loadURL('file://' + fixtures + '/api/frame-subscriber.html') + w.webContents.on('dom-ready', function () { + w.webContents.beginFrameSubscription(function (data) { + assert.notEqual(data.length, 0) + w.webContents.endFrameSubscription() + done() + }) }) }) + + it('subscribes to frame updates (only dirty rectangle)', function (done) { + this.timeout(20000) + + w.loadURL('file://' + fixtures + '/api/frame-subscriber.html') + w.webContents.on('dom-ready', function () { + w.webContents.beginFrameSubscription(true, function (data) { + assert.notEqual(data.length, 0) + w.webContents.endFrameSubscription() + done() + }) + }) + }) + + it('throws error when subscriber is not well defined', function (done) { + this.timeout(20000) + + w.loadURL('file://' + fixtures + '/api/frame-subscriber.html') + try{ + w.webContents.beginFrameSubscription(true, true) + } catch(e) { + done() + } + }) }) describe('savePage method', function () { diff --git a/spec/fixtures/api/frame-subscriber.html b/spec/fixtures/api/frame-subscriber.html new file mode 100644 index 0000000000..0d5c7b3178 --- /dev/null +++ b/spec/fixtures/api/frame-subscriber.html @@ -0,0 +1,11 @@ + + +
+ + +