diff options
| -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 |