diff options
17 files changed, 566 insertions, 162 deletions
diff --git a/content/child/notifications/notification_image_loader.cc b/content/child/notifications/notification_image_loader.cc index 4eed91f..08a108a 100644 --- a/content/child/notifications/notification_image_loader.cc +++ b/content/child/notifications/notification_image_loader.cc @@ -4,9 +4,10 @@ #include "content/child/notifications/notification_image_loader.h" +#include "base/bind.h" +#include "base/location.h" #include "base/logging.h" -#include "base/thread_task_runner_handle.h" -#include "content/child/child_thread_impl.h" +#include "base/single_thread_task_runner.h" #include "content/child/image_decoder.h" #include "third_party/WebKit/public/platform/Platform.h" #include "third_party/WebKit/public/platform/WebURL.h" @@ -23,25 +24,21 @@ namespace content { NotificationImageLoader::NotificationImageLoader( const ImageLoadCompletedCallback& callback, - const scoped_refptr<base::SingleThreadTaskRunner>& worker_task_runner) + const scoped_refptr<base::SingleThreadTaskRunner>& worker_task_runner, + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner) : callback_(callback), worker_task_runner_(worker_task_runner), - notification_id_(0), + main_task_runner_(main_task_runner), completed_(false) {} NotificationImageLoader::~NotificationImageLoader() { - if (main_thread_task_runner_) - DCHECK(main_thread_task_runner_->BelongsToCurrentThread()); + DCHECK(main_task_runner_->BelongsToCurrentThread()); } -void NotificationImageLoader::StartOnMainThread(int notification_id, - const GURL& image_url) { - DCHECK(ChildThreadImpl::current()); +void NotificationImageLoader::StartOnMainThread(const GURL& image_url) { + DCHECK(main_task_runner_->BelongsToCurrentThread()); DCHECK(!url_loader_); - main_thread_task_runner_ = base::ThreadTaskRunnerHandle::Get(); - notification_id_ = notification_id; - WebURL image_web_url(image_url); WebURLRequest request(image_web_url); request.setRequestContext(WebURLRequest::RequestContextImage); @@ -84,10 +81,9 @@ void NotificationImageLoader::RunCallbackOnWorkerThread() { SkBitmap icon = GetDecodedImage(); if (worker_task_runner_->BelongsToCurrentThread()) { - callback_.Run(notification_id_, icon); + callback_.Run(icon); } else { - worker_task_runner_->PostTask( - FROM_HERE, base::Bind(callback_, notification_id_, icon)); + worker_task_runner_->PostTask(FROM_HERE, base::Bind(callback_, icon)); } } @@ -101,8 +97,8 @@ SkBitmap NotificationImageLoader::GetDecodedImage() const { } void NotificationImageLoader::DeleteOnCorrectThread() const { - if (!ChildThreadImpl::current()) { - main_thread_task_runner_->DeleteSoon(FROM_HERE, this); + if (!main_task_runner_->BelongsToCurrentThread()) { + main_task_runner_->DeleteSoon(FROM_HERE, this); return; } diff --git a/content/child/notifications/notification_image_loader.h b/content/child/notifications/notification_image_loader.h index 33297c9..b9818d1 100644 --- a/content/child/notifications/notification_image_loader.h +++ b/content/child/notifications/notification_image_loader.h @@ -33,6 +33,10 @@ namespace content { struct NotificationImageLoaderDeleter; +// The |image| may be empty if the request failed or the image data could not be +// decoded. +using ImageLoadCompletedCallback = base::Callback<void(const SkBitmap& image)>; + // Downloads the image associated with a notification and decodes the received // image. This must be completed before notifications are shown to the user. // Image downloaders must not be re-used for multiple notifications. @@ -43,16 +47,15 @@ class NotificationImageLoader : public blink::WebURLLoaderClient, public base::RefCountedThreadSafe<NotificationImageLoader, NotificationImageLoaderDeleter> { - using ImageLoadCompletedCallback = base::Callback<void(int, const SkBitmap&)>; - public: NotificationImageLoader( const ImageLoadCompletedCallback& callback, - const scoped_refptr<base::SingleThreadTaskRunner>& worker_task_runner); + const scoped_refptr<base::SingleThreadTaskRunner>& worker_task_runner, + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner); // Asynchronously starts loading |image_url| using a Blink WebURLLoader. Must // only be called on the main thread. - void StartOnMainThread(int notification_id, const GURL& image_url); + void StartOnMainThread(const GURL& image_url); // blink::WebURLLoaderClient implementation. void didReceiveData(blink::WebURLLoader* loader, @@ -88,9 +91,8 @@ class NotificationImageLoader ImageLoadCompletedCallback callback_; scoped_refptr<base::SingleThreadTaskRunner> worker_task_runner_; - scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_; + scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_; - int notification_id_; bool completed_; scoped_ptr<blink::WebURLLoader> url_loader_; diff --git a/content/child/notifications/notification_manager.cc b/content/child/notifications/notification_manager.cc index 4cc51d7..bd948d3 100644 --- a/content/child/notifications/notification_manager.cc +++ b/content/child/notifications/notification_manager.cc @@ -32,6 +32,18 @@ int CurrentWorkerId() { return WorkerThread::GetCurrentId(); } +// Checks whether |notification_data| specifies any non-empty resources that +// need to be fetched. +bool HasResourcesToFetch(const blink::WebNotificationData& notification_data) { + if (!notification_data.icon.isEmpty()) + return true; + for (const auto& action : notification_data.actions) { + if (!action.icon.isEmpty()) + return true; + } + return false; +} + } // namespace static base::LazyInstance<base::ThreadLocalPointer<NotificationManager>>::Leaky @@ -73,13 +85,15 @@ void NotificationManager::show( const blink::WebSecurityOrigin& origin, const blink::WebNotificationData& notification_data, blink::WebNotificationDelegate* delegate) { - if (notification_data.icon.isEmpty()) { + DCHECK_EQ(0u, notification_data.actions.size()); + + if (!HasResourcesToFetch(notification_data)) { DisplayPageNotification(origin, notification_data, delegate, NotificationResources()); return; } - notifications_tracker_.FetchPageNotificationResources( + notifications_tracker_.FetchResources( notification_data, delegate, base::Bind(&NotificationManager::DisplayPageNotification, base::Unretained(this), // this owns |notifications_tracker_| @@ -92,6 +106,7 @@ void NotificationManager::showPersistent( blink::WebServiceWorkerRegistration* service_worker_registration, blink::WebNotificationShowCallbacks* callbacks) { DCHECK(service_worker_registration); + int64_t service_worker_registration_id = static_cast<WebServiceWorkerRegistrationImpl*>( service_worker_registration) @@ -116,15 +131,24 @@ void NotificationManager::showPersistent( return; } - if (notification_data.icon.isEmpty()) { + if (!HasResourcesToFetch(notification_data)) { + NotificationResources notification_resources; + + // Action indices are expected to have a corresponding icon bitmap, which + // may be empty when the developer provided no (or an invalid) icon. + if (!notification_data.actions.isEmpty()) { + notification_resources.action_icons.resize( + notification_data.actions.size()); + } + DisplayPersistentNotification( origin, notification_data, service_worker_registration_id, - std::move(owned_callbacks), NotificationResources()); + std::move(owned_callbacks), notification_resources); return; } - notifications_tracker_.FetchPersistentNotificationResources( - notification_data, + notifications_tracker_.FetchResources( + notification_data, nullptr /* delegate */, base::Bind(&NotificationManager::DisplayPersistentNotification, base::Unretained(this), // this owns |notifications_tracker_| origin, notification_data, service_worker_registration_id, @@ -159,7 +183,7 @@ void NotificationManager::getNotifications( } void NotificationManager::close(blink::WebNotificationDelegate* delegate) { - if (notifications_tracker_.CancelPageNotificationFetches(delegate)) + if (notifications_tracker_.CancelResourceFetches(delegate)) return; for (auto& iter : active_page_notifications_) { @@ -189,7 +213,7 @@ void NotificationManager::closePersistent( void NotificationManager::notifyDelegateDestroyed( blink::WebNotificationDelegate* delegate) { - if (notifications_tracker_.CancelPageNotificationFetches(delegate)) + if (notifications_tracker_.CancelResourceFetches(delegate)) return; for (auto& iter : active_page_notifications_) { @@ -306,6 +330,9 @@ void NotificationManager::DisplayPageNotification( const blink::WebNotificationData& notification_data, blink::WebNotificationDelegate* delegate, const NotificationResources& notification_resources) { + DCHECK_EQ(0u, notification_data.actions.size()); + DCHECK_EQ(0u, notification_resources.action_icons.size()); + int notification_id = notification_dispatcher_->GenerateNotificationId(CurrentWorkerId()); @@ -324,6 +351,9 @@ void NotificationManager::DisplayPersistentNotification( int64_t service_worker_registration_id, scoped_ptr<blink::WebNotificationShowCallbacks> callbacks, const NotificationResources& notification_resources) { + DCHECK_EQ(notification_data.actions.size(), + notification_resources.action_icons.size()); + // TODO(peter): GenerateNotificationId is more of a request id. Consider // renaming the method in the NotificationDispatcher if this makes sense. int request_id = diff --git a/content/child/notifications/pending_notification.cc b/content/child/notifications/pending_notification.cc new file mode 100644 index 0000000..0db2834 --- /dev/null +++ b/content/child/notifications/pending_notification.cc @@ -0,0 +1,90 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/child/notifications/pending_notification.h" + +#include "base/barrier_closure.h" +#include "base/bind.h" +#include "base/callback.h" +#include "base/location.h" +#include "base/thread_task_runner_handle.h" +#include "content/public/common/notification_resources.h" +#include "url/gurl.h" + +namespace content { + +PendingNotification::PendingNotification( + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner) + : main_task_runner_(main_task_runner), weak_factory_(this) {} + +PendingNotification::~PendingNotification() {} + +void PendingNotification::FetchResources( + const blink::WebNotificationData& notification_data, + const base::Closure& fetches_finished_callback) { + // TODO(mvanouwerkerk): Add a timeout mechanism: crbug.com/579137 + + size_t num_actions = notification_data.actions.size(); + action_icons_.resize(num_actions); + + size_t num_closures = 1 /* notification icon */ + num_actions; + fetches_finished_barrier_closure_ = + base::BarrierClosure(num_closures, fetches_finished_callback); + + FetchImageResource(notification_data.icon, + base::Bind(&PendingNotification::DidFetchNotificationIcon, + weak_factory_.GetWeakPtr())); + for (size_t i = 0; i < num_actions; i++) { + FetchImageResource(notification_data.actions[i].icon, + base::Bind(&PendingNotification::DidFetchActionIcon, + weak_factory_.GetWeakPtr(), i)); + } +} + +NotificationResources PendingNotification::GetResources() const { + NotificationResources resources; + resources.notification_icon = notification_icon_; + resources.action_icons = action_icons_; + return resources; +} + +void PendingNotification::FetchImageResource( + const blink::WebURL& image_web_url, + const ImageLoadCompletedCallback& image_callback) { + if (image_web_url.isEmpty()) { + image_callback.Run(SkBitmap()); + return; + } + + // Explicitly convert the WebURL to a GURL before passing it to a different + // thread. This is important because WebURLs must not be passed between + // threads, and per base::Bind() semantics conversion would otherwise be done + // on the receiving thread. + GURL image_gurl(image_web_url); + + scoped_refptr<NotificationImageLoader> image_loader( + new NotificationImageLoader(image_callback, + base::ThreadTaskRunnerHandle::Get(), + main_task_runner_)); + image_loaders_.push_back(image_loader); + main_task_runner_->PostTask( + FROM_HERE, base::Bind(&NotificationImageLoader::StartOnMainThread, + image_loader, image_gurl)); +} + +void PendingNotification::DidFetchNotificationIcon( + const SkBitmap& notification_icon) { + notification_icon_ = notification_icon; + fetches_finished_barrier_closure_.Run(); +} + +void PendingNotification::DidFetchActionIcon(size_t action_index, + const SkBitmap& action_icon) { + DCHECK_LT(action_index, action_icons_.size()); + + action_icons_[action_index] = action_icon; + fetches_finished_barrier_closure_.Run(); +} + +} // namespace content diff --git a/content/child/notifications/pending_notification.h b/content/child/notifications/pending_notification.h new file mode 100644 index 0000000..304cf5f --- /dev/null +++ b/content/child/notifications/pending_notification.h @@ -0,0 +1,73 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CONTENT_CHILD_NOTIFICATIONS_PENDING_NOTIFICATION_H_ +#define CONTENT_CHILD_NOTIFICATIONS_PENDING_NOTIFICATION_H_ + +#include <vector> + +#include "base/callback_forward.h" +#include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "base/memory/weak_ptr.h" +#include "content/child/notifications/notification_image_loader.h" +#include "third_party/WebKit/public/platform/WebURL.h" +#include "third_party/WebKit/public/platform/modules/notifications/WebNotificationData.h" +#include "third_party/skia/include/core/SkBitmap.h" + +namespace base { +class SingleThreadTaskRunner; +} + +namespace content { + +struct NotificationResources; + +// Stores the information associated with a pending notification, and fetches +// resources for it on the main thread. +class PendingNotification { + public: + explicit PendingNotification( + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner); + ~PendingNotification(); + + // Fetches all resources asynchronously on the main thread. + void FetchResources(const blink::WebNotificationData& notification_data, + const base::Closure& fetches_finished_callback); + + // Returns a new NotificationResources populated with the resources that have + // been fetched. + NotificationResources GetResources() const; + + private: + // Fetches an image using |image_web_url| asynchronously on the main thread. + // The |image_callback| will be called on the worker thread. + void FetchImageResource(const blink::WebURL& image_web_url, + const ImageLoadCompletedCallback& image_callback); + + // To be called on the worker thread when the notification icon has been + // fetched. + void DidFetchNotificationIcon(const SkBitmap& notification_icon); + + // To be called on the worker thread when an action icon has been fetched. + void DidFetchActionIcon(size_t action_index, const SkBitmap& action_icon); + + scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_; + + SkBitmap notification_icon_; + + std::vector<SkBitmap> action_icons_; + + base::Closure fetches_finished_barrier_closure_; + + std::vector<scoped_refptr<NotificationImageLoader>> image_loaders_; + + base::WeakPtrFactory<PendingNotification> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(PendingNotification); +}; + +} // namespace content + +#endif // CONTENT_CHILD_NOTIFICATIONS_PENDING_NOTIFICATION_H_ diff --git a/content/child/notifications/pending_notifications_tracker.cc b/content/child/notifications/pending_notifications_tracker.cc index 588af75..c57bf94 100644 --- a/content/child/notifications/pending_notifications_tracker.cc +++ b/content/child/notifications/pending_notifications_tracker.cc @@ -5,58 +5,47 @@ #include "content/child/notifications/pending_notifications_tracker.h" #include "base/bind.h" -#include "base/location.h" -#include "base/thread_task_runner_handle.h" -#include "content/child/notifications/notification_image_loader.h" +#include "base/bind_helpers.h" +#include "base/callback.h" +#include "base/memory/scoped_ptr.h" +#include "content/child/notifications/pending_notification.h" #include "content/public/common/notification_resources.h" -#include "third_party/WebKit/public/platform/modules/notifications/WebNotificationData.h" -#include "third_party/skia/include/core/SkBitmap.h" namespace content { -// Stores the information associated with a pending notification. -struct PendingNotificationsTracker::PendingNotification { - PendingNotification( - const scoped_refptr<NotificationImageLoader>& image_loader, - const NotificationResourcesFetchedCallback& callback) - : image_loader(image_loader), callback(callback) {} - - scoped_refptr<NotificationImageLoader> image_loader; - NotificationResourcesFetchedCallback callback; -}; - PendingNotificationsTracker::PendingNotificationsTracker( - scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner) - : main_thread_task_runner_(main_thread_task_runner), weak_factory_(this) {} + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner) + : next_notification_id_(0), main_task_runner_(main_task_runner) {} PendingNotificationsTracker::~PendingNotificationsTracker() {} -void PendingNotificationsTracker::FetchPageNotificationResources( +void PendingNotificationsTracker::FetchResources( const blink::WebNotificationData& notification_data, blink::WebNotificationDelegate* delegate, - const NotificationResourcesFetchedCallback& callback) { - delegate_to_pending_id_map_[delegate] = FetchNotificationResources( - notification_data, callback, - new NotificationImageLoader( - base::Bind(&PendingNotificationsTracker::DidFetchPageNotification, - weak_factory_.GetWeakPtr(), delegate), - base::ThreadTaskRunnerHandle::Get())); -} + const ResourcesCallback& resources_callback) { + int32_t notification_id = next_notification_id_++; -void PendingNotificationsTracker::FetchPersistentNotificationResources( - const blink::WebNotificationData& notification_data, - const NotificationResourcesFetchedCallback& callback) { - FetchNotificationResources( - notification_data, callback, - new NotificationImageLoader( - base::Bind( - &PendingNotificationsTracker::DidFetchPersistentNotification, - weak_factory_.GetWeakPtr()), - base::ThreadTaskRunnerHandle::Get())); + if (delegate) + delegate_to_pending_id_map_[delegate] = notification_id; + + base::Closure fetches_finished_callback = base::Bind( + &PendingNotificationsTracker::FetchesFinished, + base::Unretained(this) /* The pending notifications are owned by this. */, + delegate, notification_id, resources_callback); + + scoped_ptr<PendingNotification> pending_notification( + new PendingNotification(main_task_runner_)); + pending_notification->FetchResources(notification_data, + fetches_finished_callback); + + pending_notifications_.AddWithID(pending_notification.release(), + notification_id); } -bool PendingNotificationsTracker::CancelPageNotificationFetches( +bool PendingNotificationsTracker::CancelResourceFetches( blink::WebNotificationDelegate* delegate) { + DCHECK(delegate); + auto iter = delegate_to_pending_id_map_.find(delegate); if (iter == delegate_to_pending_id_map_.end()) return false; @@ -67,48 +56,19 @@ bool PendingNotificationsTracker::CancelPageNotificationFetches( return true; } -void PendingNotificationsTracker::DidFetchPageNotification( +void PendingNotificationsTracker::FetchesFinished( blink::WebNotificationDelegate* delegate, - int notification_id, - const SkBitmap& icon) { + int32_t notification_id, + const ResourcesCallback& resources_callback) { PendingNotification* pending_notification = pending_notifications_.Lookup(notification_id); DCHECK(pending_notification); - NotificationResources notification_resources; - notification_resources.notification_icon = icon; - pending_notification->callback.Run(notification_resources); + resources_callback.Run(pending_notification->GetResources()); - delegate_to_pending_id_map_.erase(delegate); + if (delegate) + delegate_to_pending_id_map_.erase(delegate); pending_notifications_.Remove(notification_id); } -void PendingNotificationsTracker::DidFetchPersistentNotification( - int notification_id, const SkBitmap& icon) { - PendingNotification* pending_notification = - pending_notifications_.Lookup(notification_id); - DCHECK(pending_notification); - - NotificationResources notification_resources; - notification_resources.notification_icon = icon; - pending_notification->callback.Run(notification_resources); - - pending_notifications_.Remove(notification_id); -} - -int PendingNotificationsTracker::FetchNotificationResources( - const blink::WebNotificationData& notification_data, - const NotificationResourcesFetchedCallback& callback, - const scoped_refptr<NotificationImageLoader>& image_loader) { - int notification_id = pending_notifications_.Add( - new PendingNotification(image_loader, callback)); - - main_thread_task_runner_->PostTask( - FROM_HERE, - base::Bind(&NotificationImageLoader::StartOnMainThread, image_loader, - notification_id, GURL(notification_data.icon))); - - return notification_id; -} - } // namespace content diff --git a/content/child/notifications/pending_notifications_tracker.h b/content/child/notifications/pending_notifications_tracker.h index 1dbeafe..91a7e89 100644 --- a/content/child/notifications/pending_notifications_tracker.h +++ b/content/child/notifications/pending_notifications_tracker.h @@ -7,13 +7,11 @@ #include <map> +#include "base/callback_forward.h" #include "base/id_map.h" #include "base/macros.h" #include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" -#include "base/memory/weak_ptr.h" - -class SkBitmap; +#include "content/common/content_export.h" namespace base { class SingleThreadTaskRunner; @@ -26,77 +24,58 @@ class WebNotificationDelegate; namespace content { -class NotificationImageLoader; +class PendingNotification; +class PendingNotificationsTrackerTest; struct NotificationResources; -// Type definition for the callback signature which is to be invoked when the -// resources associated with a notification have been fetched. -using NotificationResourcesFetchedCallback = - base::Callback<void(const NotificationResources&)>; - -// Tracks all aspects of all pending Web Notifications. Most notably, it's in -// charge of ensuring that all resource fetches associated with the notification -// are completed as expected. The data associated with the notifications is -// stored in this class, to maintain thread integrity of their members. -// +// Tracks all pending Web Notifications as PendingNotification instances. The +// pending notifications (and their associated data) are stored in this class. // The pending notification tracker is owned by the NotificationManager, and // lives on the thread that manager has been associated with. -class PendingNotificationsTracker { +class CONTENT_EXPORT PendingNotificationsTracker { public: explicit PendingNotificationsTracker( - scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner); + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner); ~PendingNotificationsTracker(); - // Adds a page notification to the tracker. Resource fetches for the - // notification will be started on asynchronously the main thread. - void FetchPageNotificationResources( - const blink::WebNotificationData& notification_data, - blink::WebNotificationDelegate* delegate, - const NotificationResourcesFetchedCallback& callback); + // Type definition for the callback signature which is to be invoked when the + // resources associated with a notification have been fetched. + using ResourcesCallback = base::Callback<void(const NotificationResources&)>; - // Adds a persistent notification to the tracker. Resource fetches for the - // notification will be started asynchronously on the main thread. - void FetchPersistentNotificationResources( - const blink::WebNotificationData& notification_data, - const NotificationResourcesFetchedCallback& callback); + // Adds a pending notification for which to fetch resources. The |delegate| + // may be null, but non-null values can be used to cancel the fetches. + // Resource fetches will be started asynchronously on the main thread. + void FetchResources(const blink::WebNotificationData& notification_data, + blink::WebNotificationDelegate* delegate, + const ResourcesCallback& resources_callback); - // Cancels all pending and in-fligth fetches for the page notification - // identified by |delegate|. Returns if the notification was cancelled. - bool CancelPageNotificationFetches(blink::WebNotificationDelegate* delegate); + // Cancels all pending and in-flight fetches for the notification identified + // by |delegate|, which may not be null. Returns whether the notification was + // cancelled. + bool CancelResourceFetches(blink::WebNotificationDelegate* delegate); private: - // To be called on the worker thread when the pending page notification - // identified by |notification_id| has finished fetching the icon. - void DidFetchPageNotification(blink::WebNotificationDelegate* delegate, - int notification_id, - const SkBitmap& icon); - - // To be called on the worker thread when the pending persistent notification - // identified by |notification_id| has finished fetching the icon. - void DidFetchPersistentNotification(int notification_id, - const SkBitmap& icon); - - // Common code for starting to fetch resources associated with any kind of - // notification. Will return the id of the pending notification as allocated - // in the |pending_notifications_| map. - int FetchNotificationResources( - const blink::WebNotificationData& notification_data, - const NotificationResourcesFetchedCallback& callback, - const scoped_refptr<NotificationImageLoader>& image_loader); - - scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_; - - struct PendingNotification; - - // List of the notifications whose resources are still being fetched. + friend class PendingNotificationsTrackerTest; + + // To be called on the worker thread when the pending notification + // identified by |notification_id| has finished fetching the resources. The + // |delegate| may be null. + void FetchesFinished(blink::WebNotificationDelegate* delegate, + int notification_id, + const ResourcesCallback& resources_callback); + + // Used to generate ids for tracking pending notifications. + int32_t next_notification_id_; + + scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_; + + // The notifications whose resources are still being fetched. IDMap<PendingNotification, IDMapOwnPointer> pending_notifications_; // In order to be able to cancel pending page notifications by delegate, store // a mapping of the delegate to the pending notification id as well. std::map<blink::WebNotificationDelegate*, int> delegate_to_pending_id_map_; - base::WeakPtrFactory<PendingNotificationsTracker> weak_factory_; - DISALLOW_COPY_AND_ASSIGN(PendingNotificationsTracker); }; diff --git a/content/child/notifications/pending_notifications_tracker_unittest.cc b/content/child/notifications/pending_notifications_tracker_unittest.cc new file mode 100644 index 0000000..c01917a --- /dev/null +++ b/content/child/notifications/pending_notifications_tracker_unittest.cc @@ -0,0 +1,248 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/child/notifications/pending_notifications_tracker.h" + +#include <vector> + +#include "base/base_paths.h" +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/files/file_path.h" +#include "base/files/file_util.h" +#include "base/location.h" +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/path_service.h" +#include "base/run_loop.h" +#include "base/thread_task_runner_handle.h" +#include "content/public/common/notification_resources.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/WebKit/public/platform/Platform.h" +#include "third_party/WebKit/public/platform/WebString.h" +#include "third_party/WebKit/public/platform/WebURL.h" +#include "third_party/WebKit/public/platform/WebURLError.h" +#include "third_party/WebKit/public/platform/WebURLResponse.h" +#include "third_party/WebKit/public/platform/WebUnitTestSupport.h" +#include "third_party/WebKit/public/platform/modules/notifications/WebNotificationData.h" +#include "third_party/WebKit/public/platform/modules/notifications/WebNotificationDelegate.h" +#include "third_party/skia/include/core/SkBitmap.h" +#include "url/gurl.h" + +namespace content { + +namespace { + +const char kBaseUrl[] = "http://test.com/"; +const char kIcon100x100[] = "100x100.png"; +const char kIcon110x110[] = "110x110.png"; +const char kIcon120x120[] = "120x120.png"; + +class FakeNotificationDelegate : public blink::WebNotificationDelegate { + public: + void dispatchClickEvent() override {} + void dispatchShowEvent() override {} + void dispatchErrorEvent() override {} + void dispatchCloseEvent() override {} +}; + +} // namespace + +class PendingNotificationsTrackerTest : public testing::Test { + public: + PendingNotificationsTrackerTest() + : tracker_(new PendingNotificationsTracker( + base::ThreadTaskRunnerHandle::Get())) {} + + ~PendingNotificationsTrackerTest() override { + UnitTestSupport()->unregisterAllMockedURLs(); + } + + void DidFetchResources(size_t index, const NotificationResources& resources) { + if (resources_.size() < index + 1) + resources_.resize(index + 1); + resources_[index] = resources; + } + + protected: + PendingNotificationsTracker* tracker() { return tracker_.get(); } + + size_t CountResources() { return resources_.size(); } + + NotificationResources* GetResources(size_t index) { + DCHECK_LT(index, resources_.size()); + return &resources_[index]; + } + + size_t CountPendingNotifications() { + return tracker_->pending_notifications_.size(); + } + + size_t CountDelegates() { + return tracker_->delegate_to_pending_id_map_.size(); + } + + blink::WebUnitTestSupport* UnitTestSupport() { + return blink::Platform::current()->unitTestSupport(); + } + + // Registers a mocked url. When fetched, |file_name| will be loaded from the + // test data directory. + blink::WebURL RegisterMockedURL(const std::string& file_name) { + blink::WebURL url(GURL(kBaseUrl + file_name)); + + blink::WebURLResponse response(url); + response.setMIMEType("image/png"); + response.setHTTPStatusCode(200); + + base::FilePath file_path; + base::PathService::Get(base::DIR_SOURCE_ROOT, &file_path); + file_path = file_path.Append(FILE_PATH_LITERAL("content")) + .Append(FILE_PATH_LITERAL("test")) + .Append(FILE_PATH_LITERAL("data")) + .Append(FILE_PATH_LITERAL("notifications")) + .AppendASCII(file_name); + file_path = base::MakeAbsoluteFilePath(file_path); + blink::WebString string_file_path = + blink::WebString::fromUTF8(file_path.MaybeAsASCII()); + + UnitTestSupport()->registerMockedURL(url, response, string_file_path); + + return url; + } + + // Registers a mocked url that will fail to be fetched, with a 404 error. + blink::WebURL RegisterMockedErrorURL(const std::string& file_name) { + blink::WebURL url(GURL(kBaseUrl + file_name)); + + blink::WebURLResponse response(url); + response.setMIMEType("image/png"); + response.setHTTPStatusCode(404); + + blink::WebURLError error; + error.reason = 404; + + UnitTestSupport()->registerMockedErrorURL(url, response, error); + return url; + } + + private: + base::MessageLoop message_loop_; + scoped_ptr<PendingNotificationsTracker> tracker_; + std::vector<NotificationResources> resources_; + + DISALLOW_COPY_AND_ASSIGN(PendingNotificationsTrackerTest); +}; + +TEST_F(PendingNotificationsTrackerTest, OneNotificationMultipleResources) { + blink::WebNotificationData notification_data; + notification_data.icon = RegisterMockedURL(kIcon100x100); + notification_data.actions = + blink::WebVector<blink::WebNotificationAction>(static_cast<size_t>(2)); + notification_data.actions[0].icon = RegisterMockedURL(kIcon110x110); + notification_data.actions[1].icon = RegisterMockedURL(kIcon120x120); + + tracker()->FetchResources( + notification_data, nullptr /* delegate */, + base::Bind(&PendingNotificationsTrackerTest::DidFetchResources, + base::Unretained(this), 0 /* index */)); + + ASSERT_EQ(1u, CountPendingNotifications()); + ASSERT_EQ(0u, CountResources()); + + base::RunLoop().RunUntilIdle(); + UnitTestSupport()->serveAsynchronousMockedRequests(); + + ASSERT_EQ(0u, CountPendingNotifications()); + ASSERT_EQ(1u, CountResources()); + + ASSERT_FALSE(GetResources(0u)->notification_icon.drawsNothing()); + ASSERT_EQ(100, GetResources(0u)->notification_icon.width()); + + ASSERT_EQ(2u, GetResources(0u)->action_icons.size()); + ASSERT_FALSE(GetResources(0u)->action_icons[0].drawsNothing()); + ASSERT_EQ(110, GetResources(0u)->action_icons[0].width()); + ASSERT_FALSE(GetResources(0u)->action_icons[1].drawsNothing()); + ASSERT_EQ(120, GetResources(0u)->action_icons[1].width()); +} + +TEST_F(PendingNotificationsTrackerTest, TwoNotifications) { + blink::WebNotificationData notification_data; + notification_data.icon = RegisterMockedURL(kIcon100x100); + + blink::WebNotificationData notification_data_2; + notification_data_2.icon = RegisterMockedURL(kIcon110x110); + + tracker()->FetchResources( + notification_data, nullptr /* delegate */, + base::Bind(&PendingNotificationsTrackerTest::DidFetchResources, + base::Unretained(this), 0 /* index */)); + + tracker()->FetchResources( + notification_data_2, nullptr /* delegate */, + base::Bind(&PendingNotificationsTrackerTest::DidFetchResources, + base::Unretained(this), 1 /* index */)); + + ASSERT_EQ(2u, CountPendingNotifications()); + ASSERT_EQ(0u, CountResources()); + + base::RunLoop().RunUntilIdle(); + UnitTestSupport()->serveAsynchronousMockedRequests(); + + ASSERT_EQ(0u, CountPendingNotifications()); + ASSERT_EQ(2u, CountResources()); + + ASSERT_FALSE(GetResources(0u)->notification_icon.drawsNothing()); + ASSERT_EQ(100, GetResources(0u)->notification_icon.width()); + + ASSERT_FALSE(GetResources(1u)->notification_icon.drawsNothing()); + ASSERT_EQ(110, GetResources(1u)->notification_icon.width()); +} + +TEST_F(PendingNotificationsTrackerTest, FetchError) { + blink::WebNotificationData notification_data; + notification_data.icon = RegisterMockedErrorURL(kIcon100x100); + + tracker()->FetchResources( + notification_data, nullptr /* delegate */, + base::Bind(&PendingNotificationsTrackerTest::DidFetchResources, + base::Unretained(this), 0 /* index */)); + + ASSERT_EQ(1u, CountPendingNotifications()); + ASSERT_EQ(0u, CountResources()); + + base::RunLoop().RunUntilIdle(); + UnitTestSupport()->serveAsynchronousMockedRequests(); + + ASSERT_EQ(0u, CountPendingNotifications()); + ASSERT_EQ(1u, CountResources()); + + ASSERT_TRUE(GetResources(0u)->notification_icon.drawsNothing()); +} + +TEST_F(PendingNotificationsTrackerTest, CancelFetches) { + blink::WebNotificationData notification_data; + notification_data.icon = RegisterMockedURL(kIcon100x100); + + FakeNotificationDelegate delegate; + + tracker()->FetchResources( + notification_data, &delegate, + base::Bind(&PendingNotificationsTrackerTest::DidFetchResources, + base::Unretained(this), 0 /* index */)); + + ASSERT_EQ(1u, CountPendingNotifications()); + ASSERT_EQ(1u, CountDelegates()); + ASSERT_EQ(0u, CountResources()); + + base::RunLoop().RunUntilIdle(); + tracker()->CancelResourceFetches(&delegate); + + ASSERT_EQ(0u, CountPendingNotifications()); + ASSERT_EQ(0u, CountDelegates()); + ASSERT_EQ(0u, CountResources()); +} + +} // namespace content diff --git a/content/common/platform_notification_messages.h b/content/common/platform_notification_messages.h index 58513da..91fb7c7 100644 --- a/content/common/platform_notification_messages.h +++ b/content/common/platform_notification_messages.h @@ -60,6 +60,7 @@ IPC_STRUCT_TRAITS_END() IPC_STRUCT_TRAITS_BEGIN(content::NotificationResources) IPC_STRUCT_TRAITS_MEMBER(notification_icon) + IPC_STRUCT_TRAITS_MEMBER(action_icons) IPC_STRUCT_TRAITS_END() // Messages sent from the browser to the renderer. diff --git a/content/content_child.gypi b/content/content_child.gypi index 8089133..dc179bb 100644 --- a/content/content_child.gypi +++ b/content/content_child.gypi @@ -139,6 +139,8 @@ 'child/notifications/notification_image_loader.h', 'child/notifications/notification_manager.cc', 'child/notifications/notification_manager.h', + 'child/notifications/pending_notification.cc', + 'child/notifications/pending_notification.h', 'child/notifications/pending_notifications_tracker.cc', 'child/notifications/pending_notifications_tracker.h', 'child/npapi/np_channel_base.cc', diff --git a/content/content_common.gypi b/content/content_common.gypi index b8f1727..e6ca554 100644 --- a/content/content_common.gypi +++ b/content/content_common.gypi @@ -100,6 +100,7 @@ 'public/common/mojo_shell_connection.h', 'public/common/navigator_connect_client.cc', 'public/common/navigator_connect_client.h', + 'public/common/notification_resources.cc', 'public/common/notification_resources.h', 'public/common/origin_util.h', 'public/common/page_state.cc', diff --git a/content/content_tests.gypi b/content/content_tests.gypi index f5c9f8a..7452662 100644 --- a/content/content_tests.gypi +++ b/content/content_tests.gypi @@ -635,6 +635,7 @@ 'child/indexed_db/webidbcursor_impl_unittest.cc', 'child/multipart_response_delegate_unittest.cc', 'child/notifications/notification_data_conversions_unittest.cc', + 'child/notifications/pending_notifications_tracker_unittest.cc', 'child/power_monitor_broadcast_source_unittest.cc', 'child/resource_dispatcher_unittest.cc', 'child/service_worker/service_worker_dispatcher_unittest.cc', diff --git a/content/public/common/notification_resources.cc b/content/public/common/notification_resources.cc new file mode 100644 index 0000000..b4ea9ab --- /dev/null +++ b/content/public/common/notification_resources.cc @@ -0,0 +1,13 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/public/common/notification_resources.h" + +namespace content { + +NotificationResources::NotificationResources() {} + +NotificationResources::~NotificationResources() {} + +} // namespace content diff --git a/content/public/common/notification_resources.h b/content/public/common/notification_resources.h index 8986f96..23c5477 100644 --- a/content/public/common/notification_resources.h +++ b/content/public/common/notification_resources.h @@ -5,17 +5,25 @@ #ifndef CONTENT_PUBLIC_COMMON_NOTIFICATION_RESOURCES_H_ #define CONTENT_PUBLIC_COMMON_NOTIFICATION_RESOURCES_H_ +#include <vector> + #include "content/common/content_export.h" #include "third_party/skia/include/core/SkBitmap.h" namespace content { // Structure to hold the resources associated with a Web Notification. -// TODO(mvanouwerkerk): Add resources for e.g. action icons - crbug.com/581336. struct CONTENT_EXPORT NotificationResources { + NotificationResources(); + ~NotificationResources(); + // Main icon for the notification. The bitmap may be empty if the developer // did not provide an icon, or fetching of the icon failed. SkBitmap notification_icon; + + // Icons for the actions. A bitmap may be empty if the developer did not + // provide an icon, or fetching of the icon failed. + std::vector<SkBitmap> action_icons; }; } // namespace content diff --git a/content/test/data/notifications/100x100.png b/content/test/data/notifications/100x100.png Binary files differnew file mode 100644 index 0000000..efc4081 --- /dev/null +++ b/content/test/data/notifications/100x100.png diff --git a/content/test/data/notifications/110x110.png b/content/test/data/notifications/110x110.png Binary files differnew file mode 100644 index 0000000..09ba52c --- /dev/null +++ b/content/test/data/notifications/110x110.png diff --git a/content/test/data/notifications/120x120.png b/content/test/data/notifications/120x120.png Binary files differnew file mode 100644 index 0000000..850a86e --- /dev/null +++ b/content/test/data/notifications/120x120.png |
