diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 1f441801407c..a7946786c514 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -1072,11 +1072,9 @@ void WebContents::BeginFrameSubscription( const FrameSubscriber::FrameCaptureCallback& callback) { const auto view = web_contents()->GetRenderWidgetHostView(); if (view) { - FrameSubscriber* frame_subscriber = new FrameSubscriber( - isolate(), view->GetVisibleViewportSize(), callback); - scoped_ptr del_frame_subscriber( - frame_subscriber->GetSubscriber()); - view->BeginFrameSubscription(del_frame_subscriber.Pass()); + scoped_ptr frame_subscriber(new FrameSubscriber( + isolate(), view->GetVisibleViewportSize(), callback)); + view->BeginFrameSubscription(frame_subscriber.Pass()); } } diff --git a/atom/browser/api/frame_subscriber.cc b/atom/browser/api/frame_subscriber.cc index 7d973892405c..b2b049ce8b74 100644 --- a/atom/browser/api/frame_subscriber.cc +++ b/atom/browser/api/frame_subscriber.cc @@ -13,58 +13,28 @@ namespace atom { namespace api { -using Subscriber = FrameSubscriber::Subscriber; - FrameSubscriber::FrameSubscriber(v8::Isolate* isolate, const gfx::Size& size, const FrameCaptureCallback& callback) - : isolate_(isolate), size_(size), callback_(callback), pending_frames(0) { - subscriber_ = new Subscriber(this); + : isolate_(isolate), size_(size), callback_(callback), weak_factory_(this) { } -Subscriber::Subscriber( - FrameSubscriber* frame_subscriber) : frame_subscriber_(frame_subscriber) { -} - -Subscriber::~Subscriber() { - frame_subscriber_->subscriber_ = NULL; - frame_subscriber_->RequestDestruct(); -} - -bool Subscriber::ShouldCaptureFrame( +bool FrameSubscriber::ShouldCaptureFrame( const gfx::Rect& damage_rect, base::TimeTicks present_time, scoped_refptr* storage, DeliverFrameCallback* callback) { *storage = media::VideoFrame::CreateFrame( media::PIXEL_FORMAT_YV12, - frame_subscriber_->size_, gfx::Rect(frame_subscriber_->size_), - frame_subscriber_->size_, base::TimeDelta()); + size_, gfx::Rect(size_), size_, base::TimeDelta()); *callback = base::Bind(&FrameSubscriber::OnFrameDelivered, - base::Unretained(frame_subscriber_), *storage); - frame_subscriber_->pending_frames++; + weak_factory_.GetWeakPtr(), *storage); return true; } -Subscriber* FrameSubscriber::GetSubscriber() { - return subscriber_; -} - -bool FrameSubscriber::RequestDestruct() { - bool deletable = (subscriber_ == NULL && pending_frames == 0); - // Destruct FrameSubscriber if we're not waiting for frames and the - // subscription has ended - if (deletable) - delete this; - - return deletable; -} - void FrameSubscriber::OnFrameDelivered( scoped_refptr frame, base::TimeTicks, bool result) { - pending_frames--; - - if (RequestDestruct() || subscriber_ == NULL || !result) + if (!result) return; v8::Locker locker(isolate_); diff --git a/atom/browser/api/frame_subscriber.h b/atom/browser/api/frame_subscriber.h index 6f6d66b6be80..089c4922d797 100644 --- a/atom/browser/api/frame_subscriber.h +++ b/atom/browser/api/frame_subscriber.h @@ -6,6 +6,7 @@ #define ATOM_BROWSER_API_FRAME_SUBSCRIBER_H_ #include "base/callback.h" +#include "base/memory/weak_ptr.h" #include "content/public/browser/render_widget_host_view_frame_subscriber.h" #include "ui/gfx/geometry/size.h" #include "v8/include/v8.h" @@ -14,43 +15,28 @@ namespace atom { namespace api { -class FrameSubscriber { +class FrameSubscriber : public content::RenderWidgetHostViewFrameSubscriber { public: using FrameCaptureCallback = base::Callback)>; - // Inner class that is the actual subscriber sent to chromium - class Subscriber : - public content::RenderWidgetHostViewFrameSubscriber { - public: - explicit Subscriber(FrameSubscriber* frame_subscriber); - - bool ShouldCaptureFrame(const gfx::Rect& damage_rect, - base::TimeTicks present_time, - scoped_refptr* storage, - DeliverFrameCallback* callback) override; - - ~Subscriber(); - private: - FrameSubscriber* frame_subscriber_; - }; - FrameSubscriber(v8::Isolate* isolate, const gfx::Size& size, const FrameCaptureCallback& callback); - Subscriber* GetSubscriber(); + bool ShouldCaptureFrame(const gfx::Rect& damage_rect, + base::TimeTicks present_time, + scoped_refptr* storage, + DeliverFrameCallback* callback) override; private: void OnFrameDelivered( scoped_refptr frame, base::TimeTicks, bool); - bool RequestDestruct(); - v8::Isolate* isolate_; gfx::Size size_; FrameCaptureCallback callback_; - Subscriber* subscriber_; - int pending_frames; + + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(FrameSubscriber); }; diff --git a/spec/api-auto-updater-spec.js b/spec/api-auto-updater-spec.js index 72a1d90a7fe4..efd3afe3882f 100644 --- a/spec/api-auto-updater-spec.js +++ b/spec/api-auto-updater-spec.js @@ -2,6 +2,10 @@ const assert = require('assert'); const autoUpdater = require('electron').remote.autoUpdater; const ipcRenderer = require('electron').ipcRenderer; +// Skip autoUpdater tests in MAS build. +if (process.mas) + return; + describe('autoUpdater module', function() { describe('checkForUpdates', function() { it('emits an error on Windows when called the feed URL is not set', function (done) { diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 1c92d408c4c9..89e9b770ab20 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -478,10 +478,16 @@ describe('browser-window module', function() { }); }); - xdescribe('beginFrameSubscription method', function() { + describe('beginFrameSubscription method', function() { 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();