diff options
author | cjhopman@chromium.org <cjhopman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-15 17:52:50 +0000 |
---|---|---|
committer | cjhopman@chromium.org <cjhopman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-15 17:52:50 +0000 |
commit | 703fe530a9caa267696aa7c3bc3a8dad1c4e5257 (patch) | |
tree | ac80fd8fb71764e0916e23ef52bec8f61c93cb65 | |
parent | fa944cb8e705e10d69ff6f98a8e4725a8bf34f54 (diff) | |
download | chromium_src-703fe530a9caa267696aa7c3bc3a8dad1c4e5257.zip chromium_src-703fe530a9caa267696aa7c3bc3a8dad1c4e5257.tar.gz chromium_src-703fe530a9caa267696aa7c3bc3a8dad1c4e5257.tar.bz2 |
Add DomDistillerService implementation
This adds implementation of the DomDistillerService interface. This
service receives requests to view DOM distiller entries by id or by
url and requests to save those entries. To handle a view or save
request, the DOM distiller will have to get the distilled content. If
this has previously been done, it may be available locally and the
request can be serviced by fetching the blob (currently we don't
actually store blobs, so requests are never serviced in this way). If
such a blob is not available, servicing the request will require
distilling the content on-demand.
So that multiple requests that map to the same entry don't duplicate
work, the tasks for a particular entry (fetching a blob, distilling the
url, saving, tracking viewer requests/cancels) are handled by a
TaskTracker. When all view requests have been cancelled and saving has
been completed (if requested), the TaskTracker alerts the service that
it is finished.
When the service receives a view/save request, it finds or creates a
TaskTracker for that entry and forwards the request to it. When a
TaskTracker alerts the service that its tasks have finished, the
service will (asynchronously) delete that TaskTracker.
BUG=288015
NOTRY=true
Review URL: https://codereview.chromium.org/34613011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@235368 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | components/components_tests.gyp | 6 | ||||
-rw-r--r-- | components/dom_distiller.gypi | 2 | ||||
-rw-r--r-- | components/dom_distiller/content/dom_distiller_service_factory.cc | 5 | ||||
-rw-r--r-- | components/dom_distiller/core/article_entry.h | 2 | ||||
-rw-r--r-- | components/dom_distiller/core/distiller.cc | 10 | ||||
-rw-r--r-- | components/dom_distiller/core/distiller.h | 14 | ||||
-rw-r--r-- | components/dom_distiller/core/dom_distiller_service.cc | 115 | ||||
-rw-r--r-- | components/dom_distiller/core/dom_distiller_service.h | 50 | ||||
-rw-r--r-- | components/dom_distiller/core/dom_distiller_service_unittest.cc | 229 | ||||
-rw-r--r-- | components/dom_distiller/core/fake_distiller.cc | 20 | ||||
-rw-r--r-- | components/dom_distiller/core/fake_distiller.h | 59 | ||||
-rw-r--r-- | components/dom_distiller/core/task_tracker.cc | 137 | ||||
-rw-r--r-- | components/dom_distiller/core/task_tracker.h | 115 | ||||
-rw-r--r-- | components/dom_distiller/core/task_tracker_unittest.cc | 165 |
14 files changed, 884 insertions, 45 deletions
diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 21319a2..1ec3ed5 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -23,12 +23,16 @@ 'autofill/core/common/password_form_fill_data_unittest.cc', 'browser_context_keyed_service/browser_context_dependency_manager_unittest.cc', 'browser_context_keyed_service/dependency_graph_unittest.cc', + 'dom_distiller/core/article_entry_unittest.cc', 'dom_distiller/core/distiller_unittest.cc', 'dom_distiller/core/distiller_url_fetcher_unittest.cc', 'dom_distiller/core/dom_distiller_database_unittest.cc', 'dom_distiller/core/dom_distiller_model_unittest.cc', + 'dom_distiller/core/dom_distiller_service_unittest.cc', 'dom_distiller/core/dom_distiller_store_unittest.cc', - 'dom_distiller/core/article_entry_unittest.cc', + 'dom_distiller/core/fake_distiller.cc', + 'dom_distiller/core/fake_distiller.h', + 'dom_distiller/core/task_tracker_unittest.cc', 'json_schema/json_schema_validator_unittest.cc', 'json_schema/json_schema_validator_unittest_base.cc', 'json_schema/json_schema_validator_unittest_base.h', diff --git a/components/dom_distiller.gypi b/components/dom_distiller.gypi index 74a9b1d..3febbd9 100644 --- a/components/dom_distiller.gypi +++ b/components/dom_distiller.gypi @@ -96,6 +96,8 @@ 'dom_distiller/core/dom_distiller_service.h', 'dom_distiller/core/dom_distiller_store.cc', 'dom_distiller/core/dom_distiller_store.h', + 'dom_distiller/core/task_tracker.cc', + 'dom_distiller/core/task_tracker.h', ], }, { diff --git a/components/dom_distiller/content/dom_distiller_service_factory.cc b/components/dom_distiller/content/dom_distiller_service_factory.cc index 1d030e6..a5d7e0d 100644 --- a/components/dom_distiller/content/dom_distiller_service_factory.cc +++ b/components/dom_distiller/content/dom_distiller_service_factory.cc @@ -44,9 +44,8 @@ BrowserContextKeyedService* DomDistillerServiceFactory::BuildServiceInstanceFor( new DistillerPageWebContentsFactory(profile)); scoped_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory( new DistillerURLFetcherFactory(profile->GetRequestContext())); - scoped_ptr<DistillerFactory> distiller_factory( - new DistillerFactory(distiller_page_factory.Pass(), - distiller_url_fetcher_factory.Pass())); + scoped_ptr<DistillerFactory> distiller_factory(new DistillerFactoryImpl( + distiller_page_factory.Pass(), distiller_url_fetcher_factory.Pass())); return new DomDistillerContextKeyedService(dom_distiller_store.Pass(), distiller_factory.Pass()); } diff --git a/components/dom_distiller/core/article_entry.h b/components/dom_distiller/core/article_entry.h index 388dc51..a1ffcaa 100644 --- a/components/dom_distiller/core/article_entry.h +++ b/components/dom_distiller/core/article_entry.h @@ -18,6 +18,7 @@ namespace dom_distiller { typedef sync_pb::ArticleSpecifics ArticleEntry; typedef sync_pb::ArticlePage ArticleEntryPage; +// A valid entry has an entry_id and all its pages have a URL. bool IsEntryValid(const ArticleEntry& entry); bool AreEntriesEqual(const ArticleEntry& left, const ArticleEntry& right); @@ -29,7 +30,6 @@ ArticleEntry GetEntryFromChange(const syncer::SyncChange& change); std::string GetEntryIdFromSyncData(const syncer::SyncData& data); syncer::SyncData CreateLocalData(const ArticleEntry& entry); - } // namespace dom_distiller #endif diff --git a/components/dom_distiller/core/distiller.cc b/components/dom_distiller/core/distiller.cc index 11189e5..30b0c0b 100644 --- a/components/dom_distiller/core/distiller.cc +++ b/components/dom_distiller/core/distiller.cc @@ -21,17 +21,17 @@ namespace dom_distiller { -DistillerFactory::DistillerFactory( +DistillerFactoryImpl::DistillerFactoryImpl( scoped_ptr<DistillerPageFactory> distiller_page_factory, scoped_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory) : distiller_page_factory_(distiller_page_factory.Pass()), distiller_url_fetcher_factory_(distiller_url_fetcher_factory.Pass()) {} -DistillerFactory::~DistillerFactory() {} +DistillerFactoryImpl::~DistillerFactoryImpl() {} -Distiller* DistillerFactory::CreateDistiller() { - return new DistillerImpl(*distiller_page_factory_, - *distiller_url_fetcher_factory_); +scoped_ptr<Distiller> DistillerFactoryImpl::CreateDistiller() { + return scoped_ptr<Distiller>(new DistillerImpl( + *distiller_page_factory_, *distiller_url_fetcher_factory_)); } DistillerImpl::DistillerImpl( diff --git a/components/dom_distiller/core/distiller.h b/components/dom_distiller/core/distiller.h index 6fe8228..dde179c 100644 --- a/components/dom_distiller/core/distiller.h +++ b/components/dom_distiller/core/distiller.h @@ -33,22 +33,26 @@ class Distiller { const DistillerCallback& callback) = 0; }; +class DistillerFactory { + public: + virtual scoped_ptr<Distiller> CreateDistiller() = 0; + virtual ~DistillerFactory() {} +}; // Factory for creating a Distiller. -class DistillerFactory { +class DistillerFactoryImpl : public DistillerFactory { public: - DistillerFactory( + DistillerFactoryImpl( scoped_ptr<DistillerPageFactory> distiller_page_factory, scoped_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory); - virtual ~DistillerFactory(); - virtual Distiller* CreateDistiller(); + virtual ~DistillerFactoryImpl(); + virtual scoped_ptr<Distiller> CreateDistiller() OVERRIDE; private: scoped_ptr<DistillerPageFactory> distiller_page_factory_; scoped_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory_; }; - // Distills a article from a page and associated pages. class DistillerImpl : public Distiller, public DistillerPage::Delegate { diff --git a/components/dom_distiller/core/dom_distiller_service.cc b/components/dom_distiller/core/dom_distiller_service.cc index 6f5983b..3b03b92 100644 --- a/components/dom_distiller/core/dom_distiller_service.cc +++ b/components/dom_distiller/core/dom_distiller_service.cc @@ -3,12 +3,28 @@ // found in the LICENSE file. #include "components/dom_distiller/core/dom_distiller_service.h" + +#include "base/guid.h" +#include "base/message_loop/message_loop.h" #include "components/dom_distiller/core/dom_distiller_store.h" +#include "components/dom_distiller/core/task_tracker.h" +#include "url/gurl.h" namespace dom_distiller { -ViewerHandle::ViewerHandle() {} -ViewerHandle::~ViewerHandle() {} +namespace { + +ArticleEntry CreateSkeletonEntryForUrl(const GURL& url) { + ArticleEntry skeleton; + skeleton.set_entry_id(base::GenerateGUID()); + ArticleEntryPage* page = skeleton.add_pages(); + page->set_url(url.spec()); + + DCHECK(IsEntryValid(skeleton)); + return skeleton; +} + +} // namespace DomDistillerService::DomDistillerService( scoped_ptr<DomDistillerStoreInterface> store, @@ -21,23 +37,102 @@ syncer::SyncableService* DomDistillerService::GetSyncableService() const { return store_->GetSyncableService(); } -void DomDistillerService::AddToList(const GURL& url) { NOTIMPLEMENTED(); } +void DomDistillerService::AddToList(const GURL& url) { + if (store_->GetEntryByUrl(url, NULL)) { + return; + } + TaskTracker* task_tracker = GetTaskTrackerForUrl(url); + task_tracker->SetSaveCallback(base::Bind( + &DomDistillerService::AddDistilledPageToList, base::Unretained(this))); + task_tracker->StartDistiller(distiller_factory_.get()); +} std::vector<ArticleEntry> DomDistillerService::GetEntries() const { return store_->GetEntries(); } scoped_ptr<ViewerHandle> DomDistillerService::ViewEntry( - ViewerContext* context, + ViewRequestDelegate* delegate, const std::string& entry_id) { - NOTIMPLEMENTED(); - return scoped_ptr<ViewerHandle>(); + ArticleEntry entry; + if (!store_->GetEntryById(entry_id, &entry)) { + return scoped_ptr<ViewerHandle>(); + } + + TaskTracker* task_tracker = GetTaskTrackerForEntry(entry); + scoped_ptr<ViewerHandle> viewer_handle = task_tracker->AddViewer(delegate); + task_tracker->StartDistiller(distiller_factory_.get()); + + return viewer_handle.Pass(); +} + +scoped_ptr<ViewerHandle> DomDistillerService::ViewUrl( + ViewRequestDelegate* delegate, + const GURL& url) { + if (!url.is_valid()) { + return scoped_ptr<ViewerHandle>(); + } + + TaskTracker* task_tracker = GetTaskTrackerForUrl(url); + scoped_ptr<ViewerHandle> viewer_handle = task_tracker->AddViewer(delegate); + task_tracker->StartDistiller(distiller_factory_.get()); + + return viewer_handle.Pass(); } -scoped_ptr<ViewerHandle> DomDistillerService::ViewUrl(ViewerContext* context, - const GURL& url) { - NOTIMPLEMENTED(); - return scoped_ptr<ViewerHandle>(); +TaskTracker* DomDistillerService::GetTaskTrackerForUrl(const GURL& url) { + ArticleEntry entry; + if (store_->GetEntryByUrl(url, &entry)) { + return GetTaskTrackerForEntry(entry); + } + + for (TaskList::iterator it = tasks_.begin(); it != tasks_.end(); ++it) { + if ((*it)->HasUrl(url)) { + return *it; + } + } + + ArticleEntry skeleton_entry = CreateSkeletonEntryForUrl(url); + TaskTracker* task_tracker = CreateTaskTracker(skeleton_entry); + return task_tracker; +} + +TaskTracker* DomDistillerService::GetTaskTrackerForEntry( + const ArticleEntry& entry) { + const std::string& entry_id = entry.entry_id(); + for (TaskList::iterator it = tasks_.begin(); it != tasks_.end(); ++it) { + if ((*it)->HasEntryId(entry_id)) { + return *it; + } + } + + TaskTracker* task_tracker = CreateTaskTracker(entry); + + return task_tracker; +} + +TaskTracker* DomDistillerService::CreateTaskTracker(const ArticleEntry& entry) { + TaskTracker::CancelCallback cancel_callback = + base::Bind(&DomDistillerService::CancelTask, base::Unretained(this)); + TaskTracker* tracker = new TaskTracker(entry, cancel_callback); + tasks_.push_back(tracker); + return tracker; +} + +void DomDistillerService::CancelTask(TaskTracker* task) { + TaskList::iterator it = std::find(tasks_.begin(), tasks_.end(), task); + if (it != tasks_.end()) { + tasks_.weak_erase(it); + base::MessageLoop::current()->DeleteSoon(FROM_HERE, task); + } +} + +void DomDistillerService::AddDistilledPageToList(const ArticleEntry& entry, + DistilledPageProto* proto) { + DCHECK(IsEntryValid(entry)); + DCHECK(proto); + + store_->AddEntry(entry); } } // namespace dom_distiller diff --git a/components/dom_distiller/core/dom_distiller_service.h b/components/dom_distiller/core/dom_distiller_service.h index 5227d30..6e9a18b 100644 --- a/components/dom_distiller/core/dom_distiller_service.h +++ b/components/dom_distiller/core/dom_distiller_service.h @@ -5,12 +5,12 @@ #ifndef COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_SERVICE_H_ #define COMPONENTS_DOM_DISTILLER_CORE_DOM_DISTILLER_SERVICE_H_ -#include <string> -#include <vector> - +#include "base/memory/scoped_ptr.h" +#include "base/memory/scoped_vector.h" +#include "base/memory/weak_ptr.h" #include "components/dom_distiller/core/article_entry.h" -#include "components/dom_distiller/core/distiller.h" -#include "url/gurl.h" + +class GURL; namespace syncer { class SyncableService; @@ -18,20 +18,12 @@ class SyncableService; namespace dom_distiller { +class DistilledPageProto; class DistillerFactory; class DomDistillerStoreInterface; -class ViewerContext; - -// A handle to a request to view a DOM distiller entry or URL. The request will -// be cancelled when the handle is destroyed. -class ViewerHandle { - public: - ViewerHandle(); - ~ViewerHandle(); - - private: - DISALLOW_COPY_AND_ASSIGN(ViewerHandle); -}; +class TaskTracker; +class ViewRequestDelegate; +class ViewerHandle; // Provide a view of the article list and ways of interacting with it. class DomDistillerService { @@ -50,17 +42,35 @@ class DomDistillerService { std::vector<ArticleEntry> GetEntries() const; // Request to view an article by entry id. Returns a null pointer if no entry - // with |entry_id| exists. - scoped_ptr<ViewerHandle> ViewEntry(ViewerContext* context, + // with |entry_id| exists. The ViewerHandle should be destroyed before the + // ViewRequestDelegate. The request will be cancelled when the handle is + // destroyed (or when this service is destroyed). + scoped_ptr<ViewerHandle> ViewEntry(ViewRequestDelegate* delegate, const std::string& entry_id); // Request to view an article by url. - scoped_ptr<ViewerHandle> ViewUrl(ViewerContext* context, const GURL& url); + scoped_ptr<ViewerHandle> ViewUrl(ViewRequestDelegate* delegate, + const GURL& url); private: + void CancelTask(TaskTracker* task); + void AddDistilledPageToList(const ArticleEntry& entry, + DistilledPageProto* proto); + + TaskTracker* CreateTaskTracker(const ArticleEntry& entry); + + // Gets the task tracker for the given |url| or |entry|. If no appropriate + // tracker exists, this will create one, initialize it, and add it to + // |tasks_|. + TaskTracker* GetTaskTrackerForUrl(const GURL& url); + TaskTracker* GetTaskTrackerForEntry(const ArticleEntry& entry); + scoped_ptr<DomDistillerStoreInterface> store_; scoped_ptr<DistillerFactory> distiller_factory_; + typedef ScopedVector<TaskTracker> TaskList; + TaskList tasks_; + DISALLOW_COPY_AND_ASSIGN(DomDistillerService); }; diff --git a/components/dom_distiller/core/dom_distiller_service_unittest.cc b/components/dom_distiller/core/dom_distiller_service_unittest.cc new file mode 100644 index 0000000..dc5fcda --- /dev/null +++ b/components/dom_distiller/core/dom_distiller_service_unittest.cc @@ -0,0 +1,229 @@ +// Copyright 2013 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 "components/dom_distiller/core/dom_distiller_service.h" + +#include "base/bind.h" +#include "base/containers/hash_tables.h" +#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/dom_distiller_model.h" +#include "components/dom_distiller/core/dom_distiller_store.h" +#include "components/dom_distiller/core/fake_distiller.h" +#include "components/dom_distiller/core/task_tracker.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::Invoke; +using testing::Return; +using testing::_; + +namespace dom_distiller { +namespace test { + +namespace { + +class FakeDomDistillerStore : public DomDistillerStoreInterface { + public: + virtual ~FakeDomDistillerStore() {} + + virtual bool AddEntry(const ArticleEntry& entry) OVERRIDE { + if (!IsEntryValid(entry)) { + return false; + } + + if (model_.GetEntryById(entry.entry_id(), NULL)) { + return false; + } + + syncer::SyncChangeList changes_to_apply; + syncer::SyncChangeList changes_applied; + syncer::SyncChangeList changes_missing; + + changes_to_apply.push_back(syncer::SyncChange( + FROM_HERE, syncer::SyncChange::ACTION_ADD, CreateLocalData(entry))); + + model_.ApplyChangesToModel( + changes_to_apply, &changes_applied, &changes_missing); + + return true; + } + + virtual bool GetEntryById(const std::string& entry_id, + ArticleEntry* entry) OVERRIDE { + return model_.GetEntryById(entry_id, entry); + } + + virtual bool GetEntryByUrl(const GURL& url, ArticleEntry* entry) OVERRIDE { + return model_.GetEntryByUrl(url, entry); + } + + // Gets a copy of all the current entries. + virtual std::vector<ArticleEntry> GetEntries() const OVERRIDE { + return model_.GetEntries(); + } + + virtual syncer::SyncableService* GetSyncableService() OVERRIDE { + return NULL; + } + + private: + DomDistillerModel model_; +}; + +class FakeViewRequestDelegate : public ViewRequestDelegate { + public: + virtual ~FakeViewRequestDelegate() {} + MOCK_METHOD1(OnArticleReady, void(DistilledPageProto* proto)); +}; + +} // namespace + +class DomDistillerServiceTest : public testing::Test { + public: + virtual void SetUp() { + main_loop_.reset(new base::MessageLoop()); + store_ = new FakeDomDistillerStore(); + distiller_factory_ = new MockDistillerFactory(); + service_.reset(new DomDistillerService( + scoped_ptr<DomDistillerStoreInterface>(store_), + scoped_ptr<DistillerFactory>(distiller_factory_))); + } + + virtual void TearDown() { + base::RunLoop().RunUntilIdle(); + store_ = NULL; + distiller_factory_ = NULL; + service_.reset(); + } + + protected: + FakeDomDistillerStore* store_; + MockDistillerFactory* distiller_factory_; + scoped_ptr<DomDistillerService> service_; + scoped_ptr<base::MessageLoop> main_loop_; +}; + +TEST_F(DomDistillerServiceTest, TestViewEntry) { + FakeDistiller* distiller = new FakeDistiller(); + EXPECT_CALL(*distiller_factory_, CreateDistillerImpl()) + .WillOnce(Return(distiller)); + + GURL url("http://www.example.com/p1"); + std::string entry_id("id0"); + ArticleEntry entry; + entry.set_entry_id(entry_id); + entry.add_pages()->set_url(url.spec()); + + store_->AddEntry(entry); + + FakeViewRequestDelegate viewer_delegate; + scoped_ptr<ViewerHandle> handle = + service_->ViewEntry(&viewer_delegate, entry_id); + + ASSERT_FALSE(distiller->GetCallback().is_null()); + + scoped_ptr<DistilledPageProto> proto(new DistilledPageProto); + EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get())); + + distiller->RunDistillerCallback(proto.Pass()); +} + +TEST_F(DomDistillerServiceTest, TestViewUrl) { + FakeDistiller* distiller = new FakeDistiller(); + EXPECT_CALL(*distiller_factory_, CreateDistillerImpl()) + .WillOnce(Return(distiller)); + + FakeViewRequestDelegate viewer_delegate; + GURL url("http://www.example.com/p1"); + scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url); + + ASSERT_FALSE(distiller->GetCallback().is_null()); + EXPECT_EQ(url, distiller->GetUrl()); + + scoped_ptr<DistilledPageProto> proto(new DistilledPageProto); + EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get())); + + distiller->RunDistillerCallback(proto.Pass()); +} + +TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) { + FakeDistiller* distiller = new FakeDistiller(); + FakeDistiller* distiller2 = new FakeDistiller(); + EXPECT_CALL(*distiller_factory_, CreateDistillerImpl()) + .WillOnce(Return(distiller)) + .WillOnce(Return(distiller2)); + + FakeViewRequestDelegate viewer_delegate; + FakeViewRequestDelegate viewer_delegate2; + + GURL url("http://www.example.com/p1"); + GURL url2("http://www.example.com/a/p1"); + + scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url); + scoped_ptr<ViewerHandle> handle2 = service_->ViewUrl(&viewer_delegate2, url2); + + ASSERT_FALSE(distiller->GetCallback().is_null()); + EXPECT_EQ(url, distiller->GetUrl()); + + scoped_ptr<DistilledPageProto> proto(new DistilledPageProto); + EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get())); + + distiller->RunDistillerCallback(proto.Pass()); + + ASSERT_FALSE(distiller2->GetCallback().is_null()); + EXPECT_EQ(url2, distiller2->GetUrl()); + + scoped_ptr<DistilledPageProto> proto2(new DistilledPageProto); + EXPECT_CALL(viewer_delegate2, OnArticleReady(proto2.get())); + + distiller2->RunDistillerCallback(proto2.Pass()); +} + +TEST_F(DomDistillerServiceTest, TestViewUrlCancelled) { + FakeDistiller* distiller = new FakeDistiller(); + EXPECT_CALL(*distiller_factory_, CreateDistillerImpl()) + .WillOnce(Return(distiller)); + + bool distiller_destroyed = false; + EXPECT_CALL(*distiller, Die()) + .WillOnce(testing::Assign(&distiller_destroyed, true)); + + FakeViewRequestDelegate viewer_delegate; + GURL url("http://www.example.com/p1"); + scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url); + + ASSERT_FALSE(distiller->GetCallback().is_null()); + EXPECT_EQ(url, distiller->GetUrl()); + + EXPECT_CALL(viewer_delegate, OnArticleReady(_)).Times(0); + + EXPECT_FALSE(distiller_destroyed); + + handle.reset(); + base::RunLoop().RunUntilIdle(); + EXPECT_TRUE(distiller_destroyed); +} + +TEST_F(DomDistillerServiceTest, TestAddEntry) { + FakeDistiller* distiller = new FakeDistiller(); + EXPECT_CALL(*distiller_factory_, CreateDistillerImpl()) + .WillOnce(Return(distiller)); + + GURL url("http://www.example.com/p1"); + + service_->AddToList(url); + + ASSERT_FALSE(distiller->GetCallback().is_null()); + EXPECT_EQ(url, distiller->GetUrl()); + + scoped_ptr<DistilledPageProto> proto(new DistilledPageProto); + distiller->RunDistillerCallback(proto.Pass()); + + EXPECT_TRUE(store_->GetEntryByUrl(url, NULL)); +} + +} // namespace test +} // namespace dom_distiller diff --git a/components/dom_distiller/core/fake_distiller.cc b/components/dom_distiller/core/fake_distiller.cc new file mode 100644 index 0000000..027d297 --- /dev/null +++ b/components/dom_distiller/core/fake_distiller.cc @@ -0,0 +1,20 @@ +// Copyright 2013 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 "components/dom_distiller/core/fake_distiller.h" + +namespace dom_distiller { +namespace test { + +MockDistillerFactory::MockDistillerFactory() {} +MockDistillerFactory::~MockDistillerFactory() {} + +FakeDistiller::FakeDistiller() { + EXPECT_CALL(*this, Die()).Times(testing::AnyNumber()); +} + +FakeDistiller::~FakeDistiller() { Die(); } + +} // namespace test +} // namespace dom_distiller diff --git a/components/dom_distiller/core/fake_distiller.h b/components/dom_distiller/core/fake_distiller.h new file mode 100644 index 0000000..ce6cb12 --- /dev/null +++ b/components/dom_distiller/core/fake_distiller.h @@ -0,0 +1,59 @@ +// Copyright 2013 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 COMPONENTS_DOM_DISTILLER_CORE_FAKE_DISTILLER_H_ +#define COMPONENTS_DOM_DISTILLER_CORE_FAKE_DISTILLER_H_ + +#include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/distiller.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" + +class GURL; + +namespace dom_distiller { +namespace test { + +class MockDistillerFactory : public DistillerFactory { + public: + MockDistillerFactory(); + virtual ~MockDistillerFactory(); + MOCK_METHOD0(CreateDistillerImpl, Distiller*()); + virtual scoped_ptr<Distiller> CreateDistiller() OVERRIDE { + return scoped_ptr<Distiller>(CreateDistillerImpl()); + } +}; + +class FakeDistiller : public Distiller { + public: + FakeDistiller(); + virtual ~FakeDistiller(); + MOCK_METHOD0(Die, void()); + + virtual void DistillPage(const GURL& url, + const DistillerCallback& callback) OVERRIDE { + url_ = url; + callback_ = callback; + } + + void RunDistillerCallback(scoped_ptr<DistilledPageProto> proto) { + EXPECT_FALSE(callback_.is_null()); + callback_.Run(proto.Pass()); + callback_.Reset(); + } + + GURL GetUrl() { return url_; } + + DistillerCallback GetCallback() { return callback_; } + + private: + GURL url_; + DistillerCallback callback_; +}; + +} // namespace test +} // namespace dom_distiller + +#endif // COMPONENTS_DOM_DISTILLER_CORE_FAKE_DISTILLER_H_ diff --git a/components/dom_distiller/core/task_tracker.cc b/components/dom_distiller/core/task_tracker.cc new file mode 100644 index 0000000..5a6cd7c --- /dev/null +++ b/components/dom_distiller/core/task_tracker.cc @@ -0,0 +1,137 @@ +// Copyright 2013 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 "components/dom_distiller/core/task_tracker.h" + +#include "base/message_loop/message_loop.h" + +namespace dom_distiller { + +ViewerHandle::ViewerHandle(CancelCallback callback) + : cancel_callback_(callback) {} + +ViewerHandle::~ViewerHandle() { + if (!cancel_callback_.is_null()) { + cancel_callback_.Run(); + } +} + +TaskTracker::TaskTracker(const ArticleEntry& entry, CancelCallback callback) + : cancel_callback_(callback), + entry_(entry), + distilled_page_(), + weak_ptr_factory_(this) {} + +TaskTracker::~TaskTracker() { DCHECK(viewers_.empty()); } + +void TaskTracker::StartDistiller(DistillerFactory* factory) { + if (distiller_) { + return; + } + if (entry_.pages_size() == 0) { + return; + } + + GURL url(entry_.pages(0).url()); + DCHECK(url.is_valid()); + + distiller_ = factory->CreateDistiller().Pass(); + distiller_->DistillPage(url, + base::Bind(&TaskTracker::OnDistilledDataReady, + weak_ptr_factory_.GetWeakPtr())); +} + +void TaskTracker::StartBlobFetcher() { + // TODO(cjhopman): There needs to be some local storage for the distilled + // blob. When that happens, this should start some task to fetch the blob for + // |entry_| and asynchronously notify |this| when it is done. +} + +void TaskTracker::SetSaveCallback(SaveCallback callback) { + save_callback_ = callback; + if (save_callback_.is_null()) { + MaybeCancel(); + } else if (distilled_page_) { + // Distillation for this task has already completed, and so it can be + // immediately saved. + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&TaskTracker::DoSaveCallback, + weak_ptr_factory_.GetWeakPtr())); + } +} + +scoped_ptr<ViewerHandle> TaskTracker::AddViewer(ViewRequestDelegate* delegate) { + viewers_.push_back(delegate); + if (distilled_page_) { + // Distillation for this task has already completed, and so the delegate can + // be immediately told of the result. + base::MessageLoop::current()->PostTask( + FROM_HERE, + base::Bind(&TaskTracker::NotifyViewer, + weak_ptr_factory_.GetWeakPtr(), + delegate)); + } + return scoped_ptr<ViewerHandle>(new ViewerHandle(base::Bind( + &TaskTracker::RemoveViewer, weak_ptr_factory_.GetWeakPtr(), delegate))); +} + +bool TaskTracker::HasEntryId(const std::string& entry_id) { + return entry_.entry_id() == entry_id; +} + +bool TaskTracker::HasUrl(const GURL& url) { + for (int i = 0; i < entry_.pages_size(); ++i) { + if (entry_.pages(i).url() == url.spec()) { + return true; + } + } + return false; +} + +void TaskTracker::RemoveViewer(ViewRequestDelegate* delegate) { + viewers_.erase(std::remove(viewers_.begin(), viewers_.end(), delegate)); + if (viewers_.empty()) { + MaybeCancel(); + } +} + +void TaskTracker::MaybeCancel() { + if (!save_callback_.is_null() || !viewers_.empty()) { + // There's still work to be done. + return; + } + + // The cancel callback should not delete this. To ensure that it doesn't, grab + // a weak pointer and check that it has not been invalidated. + base::WeakPtr<TaskTracker> self(weak_ptr_factory_.GetWeakPtr()); + cancel_callback_.Run(this); + DCHECK(self); +} + +void TaskTracker::DoSaveCallback() { + DCHECK(distilled_page_); + if (!save_callback_.is_null()) { + save_callback_.Run(entry_, distilled_page_.get()); + save_callback_.Reset(); + MaybeCancel(); + } +} + +void TaskTracker::NotifyViewer(ViewRequestDelegate* delegate) { + DCHECK(distilled_page_); + delegate->OnArticleReady(distilled_page_.get()); +} + +void TaskTracker::OnDistilledDataReady(scoped_ptr<DistilledPageProto> proto) { + distilled_page_ = proto.Pass(); + + entry_.set_title(distilled_page_->title()); + for (size_t i = 0; i < viewers_.size(); ++i) { + NotifyViewer(viewers_[i]); + } + DoSaveCallback(); +} + +} // namespace dom_distiller diff --git a/components/dom_distiller/core/task_tracker.h b/components/dom_distiller/core/task_tracker.h new file mode 100644 index 0000000..dbf71bf --- /dev/null +++ b/components/dom_distiller/core/task_tracker.h @@ -0,0 +1,115 @@ +// Copyright 2013 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 COMPONENTS_DOM_DISTILLER_CORE_DOM_TASK_TRACKER_H_ +#define COMPONENTS_DOM_DISTILLER_CORE_DOM_TASK_TRACKER_H_ + +#include "base/bind.h" +#include "base/memory/weak_ptr.h" +#include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/distiller.h" +#include "components/dom_distiller/core/proto/distilled_page.pb.h" + +class GURL; + +namespace dom_distiller { + +class DistilledPageProto; + +// A handle to a request to view a DOM distiller entry or URL. The request will +// be cancelled when the handle is destroyed. +class ViewerHandle { + typedef base::Callback<void()> CancelCallback; + + public: + explicit ViewerHandle(CancelCallback callback); + ~ViewerHandle(); + + private: + CancelCallback cancel_callback_; + DISALLOW_COPY_AND_ASSIGN(ViewerHandle); +}; + +// Interface for a DOM distiller entry viewer. Implement this to make a view +// request and receive the data for an entry when it becomes available. +class ViewRequestDelegate { + public: + virtual ~ViewRequestDelegate() {} + // Called when the distilled article contents are available. The + // DistilledPageProto is owned by a TaskTracker instance and is invalidated + // when the corresponding ViewerHandle is destroyed (or when the + // DomDistillerService is destroyed). + virtual void OnArticleReady(DistilledPageProto* proto) = 0; +}; + +// A TaskTracker manages the various tasks related to viewing, saving, +// distilling, and fetching an article's distilled content. +// +// There are two sources of distilled content, a Distiller and the BlobFetcher. +// At any time, at most one of each of these will be in-progress (if one +// finishes, the other will be cancelled). +// +// There are also two consumers of that content, a view request and a save +// request. There is at most one save request (i.e. we are either adding this to +// the reading list or we aren't), and may be multiple view requests. When +// the distilled content is ready, each of these requests will be notified. +// +// A view request is cancelled by deleting the corresponding ViewerHandle. Once +// all view requests are cancelled (and the save callback has been called if +// appropriate) the cancel callback will be called. +// +// After creating a TaskTracker, a consumer of distilled content should be added +// and at least one of the sources should be started. +class TaskTracker { + public: + typedef base::Callback<void(TaskTracker*)> CancelCallback; + typedef base::Callback<void(const ArticleEntry&, DistilledPageProto*)> + SaveCallback; + + TaskTracker(const ArticleEntry& entry, CancelCallback callback); + ~TaskTracker(); + + // |factory| will not be stored after this call. + void StartDistiller(DistillerFactory* factory); + void StartBlobFetcher(); + + void SetSaveCallback(SaveCallback callback); + + // The ViewerHandle should be destroyed before the ViewRequestDelegate. + scoped_ptr<ViewerHandle> AddViewer(ViewRequestDelegate* delegate); + + bool HasEntryId(const std::string& entry_id); + bool HasUrl(const GURL& url); + + private: + void OnDistilledDataReady(scoped_ptr<DistilledPageProto> distilled_page); + void DoSaveCallback(); + + void RemoveViewer(ViewRequestDelegate* delegate); + void NotifyViewer(ViewRequestDelegate* delegate); + + void MaybeCancel(); + + CancelCallback cancel_callback_; + SaveCallback save_callback_; + + scoped_ptr<Distiller> distiller_; + + // A ViewRequestDelegate will be added to this list when a view request is + // made and removed when the corresponding ViewerHandle is destroyed. + std::vector<ViewRequestDelegate*> viewers_; + + ArticleEntry entry_; + scoped_ptr<DistilledPageProto> distilled_page_; + + // Note: This should remain the last member so it'll be destroyed and + // invalidate its weak pointers before any other members are destroyed. + base::WeakPtrFactory<TaskTracker> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(TaskTracker); +}; + +} // namespace dom_distiller + +#endif // COMPONENTS_DOM_DISTILLER_CORE_TASK_TRACKER_H_ diff --git a/components/dom_distiller/core/task_tracker_unittest.cc b/components/dom_distiller/core/task_tracker_unittest.cc new file mode 100644 index 0000000..d404bf0 --- /dev/null +++ b/components/dom_distiller/core/task_tracker_unittest.cc @@ -0,0 +1,165 @@ +// Copyright 2013 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 "components/dom_distiller/core/task_tracker.h" + +#include "base/run_loop.h" +#include "components/dom_distiller/core/article_entry.h" +#include "components/dom_distiller/core/fake_distiller.h" +#include "testing/gtest/include/gtest/gtest.h" + +using testing::Return; +using testing::_; + +namespace dom_distiller { +namespace test { + +class FakeViewRequestDelegate : public ViewRequestDelegate { + public: + virtual ~FakeViewRequestDelegate() {} + MOCK_METHOD1(OnArticleReady, void(DistilledPageProto* proto)); +}; + +class TestCancelCallback { + public: + TestCancelCallback() : cancelled_(false) {} + TaskTracker::CancelCallback GetCallback() { + return base::Bind(&TestCancelCallback::Cancel, base::Unretained(this)); + } + void Cancel(TaskTracker*) { cancelled_ = true; } + bool Cancelled() { return cancelled_; } + + private: + bool cancelled_; +}; + +class MockSaveCallback { + public: + MOCK_METHOD2(Save, void(const ArticleEntry&, DistilledPageProto*)); +}; + +class DomDistillerTaskTrackerTest : public testing::Test { + public: + virtual void SetUp() OVERRIDE { + message_loop_.reset(new base::MessageLoop()); + entry_id_ = "id0"; + page_0_url_ = GURL("http://www.example.com/1"); + page_1_url_ = GURL("http://www.example.com/2"); + } + + ArticleEntry GetDefaultEntry() { + ArticleEntry entry; + entry.set_entry_id(entry_id_); + ArticleEntryPage* page0 = entry.add_pages(); + ArticleEntryPage* page1 = entry.add_pages(); + page0->set_url(page_0_url_.spec()); + page1->set_url(page_1_url_.spec()); + return entry; + } + + protected: + scoped_ptr<base::MessageLoop> message_loop_; + std::string entry_id_; + GURL page_0_url_; + GURL page_1_url_; +}; + +TEST_F(DomDistillerTaskTrackerTest, TestHasEntryId) { + MockDistillerFactory distiller_factory; + TestCancelCallback cancel_callback; + TaskTracker task_tracker(GetDefaultEntry(), cancel_callback.GetCallback()); + EXPECT_TRUE(task_tracker.HasEntryId(entry_id_)); + EXPECT_FALSE(task_tracker.HasEntryId("other_id")); +} + +TEST_F(DomDistillerTaskTrackerTest, TestHasUrl) { + MockDistillerFactory distiller_factory; + TestCancelCallback cancel_callback; + TaskTracker task_tracker(GetDefaultEntry(), cancel_callback.GetCallback()); + EXPECT_TRUE(task_tracker.HasUrl(page_0_url_)); + EXPECT_TRUE(task_tracker.HasUrl(page_1_url_)); + EXPECT_FALSE(task_tracker.HasUrl(GURL("http://other.url/"))); +} + +TEST_F(DomDistillerTaskTrackerTest, TestViewerCancelled) { + MockDistillerFactory distiller_factory; + TestCancelCallback cancel_callback; + TaskTracker task_tracker(GetDefaultEntry(), cancel_callback.GetCallback()); + + FakeViewRequestDelegate viewer_delegate; + FakeViewRequestDelegate viewer_delegate2; + scoped_ptr<ViewerHandle> handle(task_tracker.AddViewer(&viewer_delegate)); + scoped_ptr<ViewerHandle> handle2(task_tracker.AddViewer(&viewer_delegate2)); + + EXPECT_FALSE(cancel_callback.Cancelled()); + handle.reset(); + EXPECT_FALSE(cancel_callback.Cancelled()); + handle2.reset(); + EXPECT_TRUE(cancel_callback.Cancelled()); +} + +TEST_F(DomDistillerTaskTrackerTest, TestViewerCancelledWithSaveRequest) { + MockDistillerFactory distiller_factory; + TestCancelCallback cancel_callback; + TaskTracker task_tracker(GetDefaultEntry(), cancel_callback.GetCallback()); + + FakeViewRequestDelegate viewer_delegate; + scoped_ptr<ViewerHandle> handle(task_tracker.AddViewer(&viewer_delegate)); + EXPECT_FALSE(cancel_callback.Cancelled()); + + MockSaveCallback save_callback; + task_tracker.SetSaveCallback( + base::Bind(&MockSaveCallback::Save, base::Unretained(&save_callback))); + handle.reset(); + + // Since there is a pending save request, the task shouldn't be cancelled. + EXPECT_FALSE(cancel_callback.Cancelled()); +} + +TEST_F(DomDistillerTaskTrackerTest, TestViewerNotifiedOnDistillationComplete) { + MockDistillerFactory distiller_factory; + FakeDistiller* distiller = new FakeDistiller(); + EXPECT_CALL(distiller_factory, CreateDistillerImpl()) + .WillOnce(Return(distiller)); + TestCancelCallback cancel_callback; + TaskTracker task_tracker(GetDefaultEntry(), cancel_callback.GetCallback()); + + FakeViewRequestDelegate viewer_delegate; + scoped_ptr<ViewerHandle> handle(task_tracker.AddViewer(&viewer_delegate)); + base::RunLoop().RunUntilIdle(); + + EXPECT_CALL(viewer_delegate, OnArticleReady(_)); + + task_tracker.StartDistiller(&distiller_factory); + distiller->RunDistillerCallback(make_scoped_ptr(new DistilledPageProto)); + base::RunLoop().RunUntilIdle(); + + EXPECT_FALSE(cancel_callback.Cancelled()); +} + +TEST_F(DomDistillerTaskTrackerTest, + TestSaveCallbackCalledOnDistillationComplete) { + MockDistillerFactory distiller_factory; + FakeDistiller* distiller = new FakeDistiller(); + EXPECT_CALL(distiller_factory, CreateDistillerImpl()) + .WillOnce(Return(distiller)); + TestCancelCallback cancel_callback; + TaskTracker task_tracker(GetDefaultEntry(), cancel_callback.GetCallback()); + + MockSaveCallback save_callback; + task_tracker.SetSaveCallback( + base::Bind(&MockSaveCallback::Save, base::Unretained(&save_callback))); + base::RunLoop().RunUntilIdle(); + + EXPECT_CALL(save_callback, Save(_, _)); + + task_tracker.StartDistiller(&distiller_factory); + distiller->RunDistillerCallback(make_scoped_ptr(new DistilledPageProto)); + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(cancel_callback.Cancelled()); +} + +} // namespace test +} // namespace dom_distiller |