damaged -> dirty rename, fixed misc issues, changed signature, updated docs and added tests

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<FrameSubscriber> frame_subscriber(new FrameSubscriber(
-        isolate(), view, callback, only_damaged));
+        isolate(), view, callback, only_dirty));
     view->BeginFrameSubscription(std::move(frame_subscriber));
   }
 }

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<media::VideoFrame>* 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,

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<FrameSubscriber> weak_factory_;
 

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()`

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 () {

spec/fixtures/api/frame-subscriber.html

@@ -0,0 +1,11 @@
+<html>
+<body>
+  <div style="width: 10px; height: 10px;" id="dirty"></div>
+</body>
+<script type="text/javascript" charset="utf-8">
+  setInterval(function(){
+    document.getElementById('dirty').style.backgroundColor =
+      '#'+(Math.random()*0xFFFFFF<<0).toString(16)
+  }, 100)
+</script>
+</html>