summaryrefslogtreecommitdiffstats
path: root/components
diff options
context:
space:
mode:
authorshashishekhar@chromium.org <shashishekhar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-07 22:17:27 +0000
committershashishekhar@chromium.org <shashishekhar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-07 22:17:27 +0000
commit8862ccca601f63be3bfc7ce970b3ea0bc3ae4924 (patch)
treeaa571821ffe61f63913b43f18118c09cd099ae43 /components
parent978e90107e44156daf21a916aff8ff0bfca5c1bd (diff)
downloadchromium_src-8862ccca601f63be3bfc7ce970b3ea0bc3ae4924.zip
chromium_src-8862ccca601f63be3bfc7ce970b3ea0bc3ae4924.tar.gz
chromium_src-8862ccca601f63be3bfc7ce970b3ea0bc3ae4924.tar.bz2
Add incremental updates for multipage distillation.
Add support for providing incremental updates for multipage distillation. Updates are now broadcasted to the viewers, whenever a new page in the article finishes distillation. This allows viewer to show data to the user, before the distillation of the entire article is completed. This change required changing management of pages in distiller to be refcounted. BUG=288015 NOTRY=true R=cjhopman@chromium.org Review URL: https://codereview.chromium.org/178303004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@255708 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'components')
-rw-r--r--components/dom_distiller.gypi2
-rw-r--r--components/dom_distiller/content/dom_distiller_viewer_source.cc10
-rw-r--r--components/dom_distiller/content/dom_distiller_viewer_source.h2
-rw-r--r--components/dom_distiller/content/dom_distiller_viewer_source_unittest.cc5
-rw-r--r--components/dom_distiller/core/article_distillation_update.cc27
-rw-r--r--components/dom_distiller/core/article_distillation_update.h51
-rw-r--r--components/dom_distiller/core/distiller.cc53
-rw-r--r--components/dom_distiller/core/distiller.h35
-rw-r--r--components/dom_distiller/core/distiller_unittest.cc521
-rw-r--r--components/dom_distiller/core/dom_distiller_service_unittest.cc16
-rw-r--r--components/dom_distiller/core/fake_distiller.cc15
-rw-r--r--components/dom_distiller/core/fake_distiller.h12
-rw-r--r--components/dom_distiller/core/task_tracker.cc13
-rw-r--r--components/dom_distiller/core/task_tracker.h8
-rw-r--r--components/dom_distiller/core/task_tracker_unittest.cc3
15 files changed, 529 insertions, 244 deletions
diff --git a/components/dom_distiller.gypi b/components/dom_distiller.gypi
index 4e8b6ed..ba50bb7 100644
--- a/components/dom_distiller.gypi
+++ b/components/dom_distiller.gypi
@@ -50,6 +50,8 @@
'sources': [
'dom_distiller/android/component_jni_registrar.cc',
'dom_distiller/android/component_jni_registrar.h',
+ 'dom_distiller/core/article_distillation_update.cc',
+ 'dom_distiller/core/article_distillation_update.h',
'dom_distiller/core/article_entry.cc',
'dom_distiller/core/article_entry.h',
'dom_distiller/core/distiller.cc',
diff --git a/components/dom_distiller/content/dom_distiller_viewer_source.cc b/components/dom_distiller/content/dom_distiller_viewer_source.cc
index 56aa0e8..4538eb5 100644
--- a/components/dom_distiller/content/dom_distiller_viewer_source.cc
+++ b/components/dom_distiller/content/dom_distiller_viewer_source.cc
@@ -59,6 +59,9 @@ class DomDistillerViewerSource::RequestViewerHandle
virtual void OnArticleReady(const DistilledArticleProto* article_proto)
OVERRIDE;
+ virtual void OnArticleUpdated(ArticleDistillationUpdate article_update)
+ OVERRIDE;
+
void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle);
private:
@@ -102,6 +105,11 @@ void DomDistillerViewerSource::RequestViewerHandle::OnArticleReady(
base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
}
+void DomDistillerViewerSource::RequestViewerHandle::OnArticleUpdated(
+ ArticleDistillationUpdate article_update) {
+ // TODO(nyquist): Add support for displaying pages incrementally.
+}
+
void DomDistillerViewerSource::RequestViewerHandle::TakeViewerHandle(
scoped_ptr<ViewerHandle> viewer_handle) {
viewer_handle_ = viewer_handle.Pass();
@@ -179,7 +187,7 @@ bool DomDistillerViewerSource::ShouldServiceRequest(
// TODO(nyquist): Start tracking requests using this method.
void DomDistillerViewerSource::WillServiceRequest(
const net::URLRequest* request,
- std::string* path) const {};
+ std::string* path) const {}
std::string DomDistillerViewerSource::GetContentSecurityPolicyObjectSrc()
const {
diff --git a/components/dom_distiller/content/dom_distiller_viewer_source.h b/components/dom_distiller/content/dom_distiller_viewer_source.h
index 2373893..95513db 100644
--- a/components/dom_distiller/content/dom_distiller_viewer_source.h
+++ b/components/dom_distiller/content/dom_distiller_viewer_source.h
@@ -5,6 +5,8 @@
#ifndef COMPONENTS_DOM_DISTILLER_CONTENT_DOM_DISTILLER_VIEWER_SOURCE_H_
#define COMPONENTS_DOM_DISTILLER_CONTENT_DOM_DISTILLER_VIEWER_SOURCE_H_
+#include <string>
+
#include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "content/public/browser/url_data_source.h"
diff --git a/components/dom_distiller/content/dom_distiller_viewer_source_unittest.cc b/components/dom_distiller/content/dom_distiller_viewer_source_unittest.cc
index 3c4ee7b..7e5db19 100644
--- a/components/dom_distiller/content/dom_distiller_viewer_source_unittest.cc
+++ b/components/dom_distiller/content/dom_distiller_viewer_source_unittest.cc
@@ -4,7 +4,10 @@
#include "components/dom_distiller/content/dom_distiller_viewer_source.h"
+#include <vector>
+
#include "base/callback.h"
+#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/article_entry.h"
#include "components/dom_distiller/core/dom_distiller_service.h"
#include "components/dom_distiller/core/dom_distiller_store.h"
@@ -23,6 +26,8 @@ class FakeViewRequestDelegate : public ViewRequestDelegate {
public:
virtual ~FakeViewRequestDelegate() {}
MOCK_METHOD1(OnArticleReady, void(const DistilledArticleProto* proto));
+ MOCK_METHOD1(OnArticleUpdated,
+ void(ArticleDistillationUpdate article_update));
};
class TestDomDistillerService : public DomDistillerServiceInterface {
diff --git a/components/dom_distiller/core/article_distillation_update.cc b/components/dom_distiller/core/article_distillation_update.cc
new file mode 100644
index 0000000..1ed54b5
--- /dev/null
+++ b/components/dom_distiller/core/article_distillation_update.cc
@@ -0,0 +1,27 @@
+// Copyright 2014 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/article_distillation_update.h"
+
+#include "base/logging.h"
+
+namespace dom_distiller {
+
+ArticleDistillationUpdate::ArticleDistillationUpdate(
+ const std::vector<scoped_refptr<RefCountedPageProto> >& pages,
+ bool has_next_page,
+ bool has_prev_page)
+ : has_next_page_(has_next_page),
+ has_prev_page_(has_prev_page),
+ pages_(pages) {}
+
+ArticleDistillationUpdate::~ArticleDistillationUpdate() {}
+
+const DistilledPageProto& ArticleDistillationUpdate::GetDistilledPage(
+ size_t index) const {
+ DCHECK_GT(pages_.size(), index);
+ return pages_[index]->data;
+}
+
+} // namespace dom_distiller
diff --git a/components/dom_distiller/core/article_distillation_update.h b/components/dom_distiller/core/article_distillation_update.h
new file mode 100644
index 0000000..f6de861
--- /dev/null
+++ b/components/dom_distiller/core/article_distillation_update.h
@@ -0,0 +1,51 @@
+// Copyright 2014 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_ARTICLE_DISTILLATION_UPDATE_H_
+#define COMPONENTS_DOM_DISTILLER_CORE_ARTICLE_DISTILLATION_UPDATE_H_
+
+#include <vector>
+
+#include "base/memory/ref_counted.h"
+#include "components/dom_distiller/core/proto/distilled_page.pb.h"
+
+namespace dom_distiller {
+
+// Update about an article that is currently under distillation.
+class ArticleDistillationUpdate {
+ public:
+ typedef base::RefCountedData<DistilledPageProto> RefCountedPageProto;
+
+ ArticleDistillationUpdate(
+ const std::vector<scoped_refptr<RefCountedPageProto> >& pages,
+ bool has_next_page,
+ bool has_prev_page);
+ ~ArticleDistillationUpdate();
+
+ // Returns the distilled page at |index|.
+ const DistilledPageProto& GetDistilledPage(size_t index) const;
+
+ // Returns the size of distilled pages in this update.
+ size_t GetPagesSize() const { return pages_.size(); }
+
+ // Returns true, if article has a next page that is currently under
+ // distillation and that is not part of the distilled pages included in this
+ // update.
+ bool HasNextPage() const { return has_next_page_; }
+
+ // Returns true if article has a previous page that is currently under
+ // distillation and that is not part of the distilled pages included in this
+ // update.
+ bool HasPrevPage() const { return has_prev_page_; }
+
+ private:
+ bool has_next_page_;
+ bool has_prev_page_;
+ // Currently available pages.
+ std::vector<scoped_refptr<RefCountedPageProto> > pages_;
+};
+
+} // namespace dom_distiller
+
+#endif // COMPONENTS_DOM_DISTILLER_CORE_ARTICLE_DISTILLATION_UPDATE_H_
diff --git a/components/dom_distiller/core/distiller.cc b/components/dom_distiller/core/distiller.cc
index 8bdc905..243b2796 100644
--- a/components/dom_distiller/core/distiller.cc
+++ b/components/dom_distiller/core/distiller.cc
@@ -5,6 +5,7 @@
#include "components/dom_distiller/core/distiller.h"
#include <map>
+#include <vector>
#include "base/bind.h"
#include "base/callback.h"
@@ -96,9 +97,11 @@ DistillerImpl::DistilledPageData* DistillerImpl::GetPageAtIndex(size_t index)
}
void DistillerImpl::DistillPage(const GURL& url,
- const DistillerCallback& distillation_cb) {
+ const DistillationFinishedCallback& finished_cb,
+ const DistillationUpdateCallback& update_cb) {
DCHECK(AreAllPagesFinished());
- distillation_cb_ = distillation_cb;
+ finished_cb_ = finished_cb;
+ update_cb_ = update_cb;
AddToDistillationQueue(0, url);
DistillNextPage();
@@ -136,13 +139,13 @@ void DistillerImpl::OnPageDistillationFinished(
if (distillation_successful) {
DistilledPageData* page_data =
GetPageAtIndex(started_pages_index_[page_num]);
- DistilledPageProto* current_page = new DistilledPageProto();
- page_data->proto.reset(current_page);
+ page_data->distilled_page_proto =
+ new base::RefCountedData<DistilledPageProto>();
page_data->page_num = page_num;
page_data->title = distilled_page->title;
- current_page->set_url(page_url.spec());
- current_page->set_html(distilled_page->html);
+ page_data->distilled_page_proto->data.set_url(page_url.spec());
+ page_data->distilled_page_proto->data.set_html(distilled_page->html);
GURL next_page_url(distilled_page->next_page_url);
if (next_page_url.is_valid()) {
@@ -195,7 +198,7 @@ void DistillerImpl::OnFetchImageDone(int page_num,
const std::string& response) {
DCHECK(started_pages_index_.find(page_num) != started_pages_index_.end());
DistilledPageData* page_data = GetPageAtIndex(started_pages_index_[page_num]);
- DCHECK(page_data->proto);
+ DCHECK(page_data->distilled_page_proto);
DCHECK(url_fetcher);
ScopedVector<DistillerURLFetcher>::iterator fetcher_it =
std::find(page_data->image_fetchers_.begin(),
@@ -208,7 +211,8 @@ void DistillerImpl::OnFetchImageDone(int page_num,
page_data->image_fetchers_.weak_erase(fetcher_it);
base::MessageLoop::current()->DeleteSoon(FROM_HERE, url_fetcher);
- DistilledPageProto_Image* image = page_data->proto->add_image();
+ DistilledPageProto_Image* image =
+ page_data->distilled_page_proto->data.add_image();
image->set_name(id);
image->set_data(response);
@@ -222,12 +226,37 @@ void DistillerImpl::AddPageIfDone(int page_num) {
if (page_data->image_fetchers_.empty()) {
finished_pages_index_[page_num] = started_pages_index_[page_num];
started_pages_index_.erase(page_num);
+ const ArticleDistillationUpdate& article_update =
+ CreateDistillationUpdate();
+ DCHECK_EQ(article_update.GetPagesSize(), finished_pages_index_.size());
+ update_cb_.Run(article_update);
RunDistillerCallbackIfDone();
}
}
+const ArticleDistillationUpdate DistillerImpl::CreateDistillationUpdate()
+ const {
+ bool has_prev_page = false;
+ bool has_next_page = false;
+ if (!finished_pages_index_.empty()) {
+ int prev_page_num = finished_pages_index_.begin()->first - 1;
+ int next_page_num = finished_pages_index_.rbegin()->first + 1;
+ has_prev_page = IsPageNumberInUse(prev_page_num);
+ has_next_page = IsPageNumberInUse(next_page_num);
+ }
+
+ std::vector<scoped_refptr<ArticleDistillationUpdate::RefCountedPageProto> >
+ update_pages;
+ for (std::map<int, size_t>::const_iterator it = finished_pages_index_.begin();
+ it != finished_pages_index_.end();
+ ++it) {
+ update_pages.push_back(pages_[it->second]->distilled_page_proto);
+ }
+ return ArticleDistillationUpdate(update_pages, has_next_page, has_prev_page);
+}
+
void DistillerImpl::RunDistillerCallbackIfDone() {
- DCHECK(!distillation_cb_.is_null());
+ DCHECK(!finished_cb_.is_null());
if (AreAllPagesFinished()) {
bool first_page = true;
scoped_ptr<DistilledArticleProto> article_proto(
@@ -236,7 +265,7 @@ void DistillerImpl::RunDistillerCallbackIfDone() {
for (std::map<int, size_t>::iterator it = finished_pages_index_.begin();
it != finished_pages_index_.end();) {
DistilledPageData* page_data = GetPageAtIndex(it->second);
- *(article_proto->add_pages()) = *(page_data->proto);
+ *(article_proto->add_pages()) = page_data->distilled_page_proto->data;
if (first_page) {
article_proto->set_title(page_data->title);
@@ -252,8 +281,8 @@ void DistillerImpl::RunDistillerCallbackIfDone() {
DCHECK(pages_.empty());
DCHECK(finished_pages_index_.empty());
- distillation_cb_.Run(article_proto.Pass());
- distillation_cb_.Reset();
+ finished_cb_.Run(article_proto.Pass());
+ finished_cb_.Reset();
}
}
diff --git a/components/dom_distiller/core/distiller.h b/components/dom_distiller/core/distiller.h
index 0b3a49f..082dcad 100644
--- a/components/dom_distiller/core/distiller.h
+++ b/components/dom_distiller/core/distiller.h
@@ -5,12 +5,15 @@
#ifndef COMPONENTS_DOM_DISTILLER_CORE_DISTILLER_H_
#define COMPONENTS_DOM_DISTILLER_CORE_DISTILLER_H_
+#include <map>
#include <string>
#include "base/callback.h"
#include "base/containers/hash_tables.h"
+#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
+#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/distiller_url_fetcher.h"
#include "components/dom_distiller/core/page_distiller.h"
#include "components/dom_distiller/core/proto/distilled_article.pb.h"
@@ -24,13 +27,21 @@ class DistillerImpl;
class Distiller {
public:
typedef base::Callback<void(scoped_ptr<DistilledArticleProto>)>
- DistillerCallback;
+ DistillationFinishedCallback;
+ typedef base::Callback<void(const ArticleDistillationUpdate&)>
+ DistillationUpdateCallback;
+
virtual ~Distiller() {}
- // Distills a page, and asynchrounously returns the article HTML to the
- // supplied callback.
+ // Distills a page, and asynchronously returns the article HTML to the
+ // supplied |finished_cb| callback. |update_cb| is invoked whenever article
+ // under distillation is updated with more data.
+ // E.g. when distilling a 2 page article, |update_cb| may be invoked each time
+ // a distilled page is added and |finished_cb| will be invoked once
+ // distillation is completed.
virtual void DistillPage(const GURL& url,
- const DistillerCallback& callback) = 0;
+ const DistillationFinishedCallback& finished_cb,
+ const DistillationUpdateCallback& update_cb) = 0;
};
class DistillerFactory {
@@ -66,7 +77,9 @@ class DistillerImpl : public Distiller {
virtual void Init();
virtual void DistillPage(const GURL& url,
- const DistillerCallback& callback) OVERRIDE;
+ const DistillationFinishedCallback& finished_cb,
+ const DistillationUpdateCallback& update_cb)
+ OVERRIDE;
void SetMaxNumPagesInArticle(size_t max_num_pages);
@@ -84,7 +97,8 @@ class DistillerImpl : public Distiller {
int page_num;
std::string title;
ScopedVector<DistillerURLFetcher> image_fetchers_;
- scoped_ptr<DistilledPageProto> proto;
+ scoped_refptr<base::RefCountedData<DistilledPageProto> >
+ distilled_page_proto;
private:
DISALLOW_COPY_AND_ASSIGN(DistilledPageData);
@@ -122,7 +136,7 @@ class DistillerImpl : public Distiller {
// includes pages that are pending distillation.
size_t TotalPageCount() const;
- // Runs |distillation_cb_| if all distillation callbacks and image fetches are
+ // Runs |finished_cb_| if all distillation callbacks and image fetches are
// complete.
void RunDistillerCallbackIfDone();
@@ -132,9 +146,14 @@ class DistillerImpl : public Distiller {
DistilledPageData* GetPageAtIndex(size_t index) const;
+ // Create an ArticleDistillationUpdate for the current distillation
+ // state.
+ const ArticleDistillationUpdate CreateDistillationUpdate() const;
+
const DistillerURLFetcherFactory& distiller_url_fetcher_factory_;
scoped_ptr<PageDistiller> page_distiller_;
- DistillerCallback distillation_cb_;
+ DistillationFinishedCallback finished_cb_;
+ DistillationUpdateCallback update_cb_;
// Set of pages that are under distillation or have finished distillation.
// |started_pages_index_| and |finished_pages_index_| maintains the mapping
diff --git a/components/dom_distiller/core/distiller_unittest.cc b/components/dom_distiller/core/distiller_unittest.cc
index d3467c7..24647a42 100644
--- a/components/dom_distiller/core/distiller_unittest.cc
+++ b/components/dom_distiller/core/distiller_unittest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <algorithm>
#include <map>
#include <string>
#include <vector>
@@ -13,6 +14,7 @@
#include "base/message_loop/message_loop.h"
#include "base/strings/string_number_conversions.h"
#include "base/values.h"
+#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/distiller.h"
#include "components/dom_distiller/core/distiller_page.h"
#include "components/dom_distiller/core/proto/distilled_article.pb.h"
@@ -23,40 +25,157 @@
using std::vector;
using std::string;
-using::testing::Invoke;
-using::testing::Return;
-using::testing::_;
+using ::testing::Invoke;
+using ::testing::Return;
+using ::testing::_;
namespace {
- const char kTitle[] = "Title";
- const char kContent[] = "Content";
- const char kURL[] = "http://a.com/";
- const size_t kTotalImages = 2;
- const char* kImageURLs[kTotalImages] = {"http://a.com/img1.jpg",
- "http://a.com/img2.jpg"};
- const char* kImageData[kTotalImages] = {"abcde", "12345"};
-
- const string GetImageName(int page_num, int image_num) {
- return base::IntToString(page_num) + "_" + base::IntToString(image_num);
+const char kTitle[] = "Title";
+const char kContent[] = "Content";
+const char kURL[] = "http://a.com/";
+const size_t kTotalImages = 2;
+const char* kImageURLs[kTotalImages] = {"http://a.com/img1.jpg",
+ "http://a.com/img2.jpg"};
+const char* kImageData[kTotalImages] = {"abcde", "12345"};
+
+const string GetImageName(int page_num, int image_num) {
+ return base::IntToString(page_num) + "_" + base::IntToString(image_num);
+}
+
+scoped_ptr<base::ListValue> CreateDistilledValueReturnedFromJS(
+ const string& title,
+ const string& content,
+ const vector<int>& image_indices,
+ const string& next_page_url,
+ const string& prev_page_url = "") {
+ scoped_ptr<base::ListValue> list(new base::ListValue());
+
+ list->AppendString(title);
+ list->AppendString(content);
+ list->AppendString(next_page_url);
+ list->AppendString(prev_page_url);
+ for (size_t i = 0; i < image_indices.size(); ++i) {
+ list->AppendString(kImageURLs[image_indices[i]]);
}
+ return list.Pass();
+}
+
+// Return the sequence in which Distiller will distill pages.
+// Note: ignores any delays due to fetching images etc.
+vector<int> GetPagesInSequence(int start_page_num, int num_pages) {
+ // Distiller prefers distilling past pages first. E.g. when distillation
+ // starts on page 2 then pages are distilled in the order: 2, 1, 0, 3, 4.
+ vector<int> page_nums;
+ for (int page = start_page_num; page >= 0; --page)
+ page_nums.push_back(page);
+ for (int page = start_page_num + 1; page < num_pages; ++page)
+ page_nums.push_back(page);
+ return page_nums;
+}
+
+struct MultipageDistillerData {
+ public:
+ MultipageDistillerData() {}
+ ~MultipageDistillerData() {}
+ vector<string> page_urls;
+ vector<string> content;
+ vector<vector<int> > image_ids;
+ // The Javascript values returned by mock distiller.
+ ScopedVector<base::Value> distilled_values;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MultipageDistillerData);
+};
- scoped_ptr<base::ListValue> CreateDistilledValueReturnedFromJS(
- const string& title,
- const string& content,
- const vector<int>& image_indices,
- const string& next_page_url,
- const string& prev_page_url = "") {
- scoped_ptr<base::ListValue> list(new base::ListValue());
-
- list->AppendString(title);
- list->AppendString(content);
- list->AppendString(next_page_url);
- list->AppendString(prev_page_url);
- for (size_t i = 0; i < image_indices.size(); ++i) {
- list->AppendString(kImageURLs[image_indices[i]]);
+void VerifyIncrementalUpdatesMatch(
+ const MultipageDistillerData* distiller_data,
+ int num_pages_in_article,
+ const vector<dom_distiller::ArticleDistillationUpdate>& incremental_updates,
+ int start_page_num) {
+ vector<int> page_seq =
+ GetPagesInSequence(start_page_num, num_pages_in_article);
+ // Updates should contain a list of pages. Pages in an update should be in
+ // the correct ascending page order regardless of |start_page_num|.
+ // E.g. if distillation starts at page 2 of a 3 page article, the updates
+ // will be [[2], [1, 2], [1, 2, 3]]. This example assumes that image fetches
+ // do not delay distillation of a page. There can be scenarios when image
+ // fetch delays distillation of a page (E.g. 1 is delayed due to image
+ // fetches so updates can be in this order [[2], [2,3], [1,2,3]].
+ for (size_t update_count = 0; update_count < incremental_updates.size();
+ ++update_count) {
+ const dom_distiller::ArticleDistillationUpdate& update =
+ incremental_updates[update_count];
+ EXPECT_EQ(update_count + 1, update.GetPagesSize());
+
+ vector<int> expected_page_nums_in_update(
+ page_seq.begin(), page_seq.begin() + update.GetPagesSize());
+ std::sort(expected_page_nums_in_update.begin(),
+ expected_page_nums_in_update.end());
+
+ // If we already got the first page then there is no previous page.
+ EXPECT_EQ((expected_page_nums_in_update[0] != 0), update.HasPrevPage());
+
+ // if we already got the last page then there is no next page.
+ EXPECT_EQ(
+ (*expected_page_nums_in_update.rbegin()) != num_pages_in_article - 1,
+ update.HasNextPage());
+ for (size_t j = 0; j < update.GetPagesSize(); ++j) {
+ int actual_page_num = expected_page_nums_in_update[j];
+ EXPECT_EQ(distiller_data->page_urls[actual_page_num],
+ update.GetDistilledPage(j).url());
+ EXPECT_EQ(distiller_data->content[actual_page_num],
+ update.GetDistilledPage(j).html());
}
- return list.Pass();
}
+}
+
+scoped_ptr<MultipageDistillerData> CreateMultipageDistillerDataWithoutImages(
+ size_t pages_size) {
+ scoped_ptr<MultipageDistillerData> result(new MultipageDistillerData());
+ string url_prefix = "http://a.com/";
+ for (size_t page_num = 0; page_num < pages_size; ++page_num) {
+ result->page_urls.push_back(url_prefix + base::IntToString(page_num));
+ result->content.push_back("Content for page:" +
+ base::IntToString(page_num));
+ result->image_ids.push_back(vector<int>());
+ string next_page_url = (page_num + 1 < pages_size)
+ ? url_prefix + base::IntToString(page_num + 1)
+ : "";
+ string prev_page_url =
+ (page_num > 0) ? result->page_urls[page_num - 1] : "";
+ scoped_ptr<base::ListValue> distilled_value =
+ CreateDistilledValueReturnedFromJS(kTitle,
+ result->content[page_num],
+ result->image_ids[page_num],
+ next_page_url,
+ prev_page_url);
+ result->distilled_values.push_back(distilled_value.release());
+ }
+ return result.Pass();
+}
+
+void VerifyArticleProtoMatchesMultipageData(
+ const dom_distiller::DistilledArticleProto* article_proto,
+ const MultipageDistillerData* distiller_data,
+ size_t pages_size) {
+ EXPECT_EQ(pages_size, static_cast<size_t>(article_proto->pages_size()));
+ EXPECT_EQ(kTitle, article_proto->title());
+ for (size_t page_num = 0; page_num < pages_size; ++page_num) {
+ const dom_distiller::DistilledPageProto& page =
+ article_proto->pages(page_num);
+ EXPECT_EQ(distiller_data->content[page_num], page.html());
+ EXPECT_EQ(distiller_data->page_urls[page_num], page.url());
+ EXPECT_EQ(distiller_data->image_ids[page_num].size(),
+ static_cast<size_t>(page.image_size()));
+ const vector<int>& image_ids_for_page = distiller_data->image_ids[page_num];
+ for (size_t img_num = 0; img_num < image_ids_for_page.size(); ++img_num) {
+ EXPECT_EQ(kImageData[image_ids_for_page[img_num]],
+ page.image(img_num).data());
+ EXPECT_EQ(GetImageName(page_num + 1, img_num),
+ page.image(img_num).name());
+ }
+ }
+}
} // namespace
@@ -79,13 +198,14 @@ class TestDistillerURLFetcher : public DistillerURLFetcher {
base::MessageLoop::current()->PostTask(
FROM_HERE,
base::Bind(&TestDistillerURLFetcher::CallCallback,
- base::Unretained(this), url, callback));
+ base::Unretained(this),
+ url,
+ callback));
}
std::map<string, string> responses_;
};
-
class TestDistillerURLFetcherFactory : public DistillerURLFetcherFactory {
public:
TestDistillerURLFetcherFactory() : DistillerURLFetcherFactory(NULL) {}
@@ -95,7 +215,6 @@ class TestDistillerURLFetcherFactory : public DistillerURLFetcherFactory {
}
};
-
class MockDistillerPage : public DistillerPage {
public:
MOCK_METHOD0(InitImpl, void());
@@ -106,7 +225,6 @@ class MockDistillerPage : public DistillerPage {
: DistillerPage(delegate) {}
};
-
class MockDistillerPageFactory : public DistillerPageFactory {
public:
MOCK_CONST_METHOD1(
@@ -122,13 +240,26 @@ class MockDistillerPageFactory : public DistillerPageFactory {
class DistillerTest : public testing::Test {
public:
virtual ~DistillerTest() {}
- void OnDistillPageDone(scoped_ptr<DistilledArticleProto> proto) {
+ void OnDistillArticleDone(scoped_ptr<DistilledArticleProto> proto) {
article_proto_ = proto.Pass();
}
+ void OnDistillArticleUpdate(const ArticleDistillationUpdate& article_update) {
+ in_sequence_updates_.push_back(article_update);
+ }
+
+ void DistillPage(const std::string& url) {
+ distiller_->DistillPage(GURL(url),
+ base::Bind(&DistillerTest::OnDistillArticleDone,
+ base::Unretained(this)),
+ base::Bind(&DistillerTest::OnDistillArticleUpdate,
+ base::Unretained(this)));
+ }
+
protected:
scoped_ptr<DistillerImpl> distiller_;
scoped_ptr<DistilledArticleProto> article_proto_;
+ std::vector<ArticleDistillationUpdate> in_sequence_updates_;
MockDistillerPageFactory page_factory_;
TestDistillerURLFetcherFactory url_fetcher_factory_;
};
@@ -149,29 +280,25 @@ ACTION_P2(CreateMockDistillerPage, list, kurl) {
return distiller_page;
}
-ACTION_P4(CreateMockDistillerPages, lists, kurls, num_pages, start_page_num) {
+ACTION_P3(CreateMockDistillerPages,
+ distiller_data,
+ pages_size,
+ start_page_num) {
DistillerPage::Delegate* delegate = arg0;
MockDistillerPage* distiller_page = new MockDistillerPage(delegate);
EXPECT_CALL(*distiller_page, InitImpl());
{
testing::InSequence s;
- // Distiller prefers distilling past pages first. E.g. when distillation
- // starts on page 2 then pages are distilled in the order: 2, 1, 0, 3, 4.
- vector<int> page_nums;
- for (int page = start_page_num; page >= 0; --page)
- page_nums.push_back(page);
- for (int page = start_page_num + 1; page < num_pages; ++page)
- page_nums.push_back(page);
-
- for (size_t page_num = 0; page_num < page_nums.size(); ++page_num) {
+ vector<int> page_nums = GetPagesInSequence(start_page_num, pages_size);
+ for (size_t page_num = 0; page_num < pages_size; ++page_num) {
int page = page_nums[page_num];
- GURL url = GURL(kurls[page]);
+ GURL url = GURL(distiller_data->page_urls[page]);
EXPECT_CALL(*distiller_page, LoadURLImpl(url))
.WillOnce(testing::InvokeWithoutArgs(distiller_page,
&DistillerPage::OnLoadURLDone));
EXPECT_CALL(*distiller_page, ExecuteJavaScriptImpl(_))
.WillOnce(DistillerPageOnExecuteJavaScriptDone(
- distiller_page, url, lists[page].get()));
+ distiller_page, url, distiller_data->distilled_values[page]));
}
}
return distiller_page;
@@ -185,9 +312,7 @@ TEST_F(DistillerTest, DistillPage) {
.WillOnce(CreateMockDistillerPage(list.get(), GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
distiller_->Init();
- distiller_->DistillPage(
- GURL(kURL),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+ DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
EXPECT_EQ(article_proto_->pages_size(), 1);
@@ -203,14 +328,11 @@ TEST_F(DistillerTest, DistillPageWithImages) {
image_indices.push_back(1);
scoped_ptr<base::ListValue> list =
CreateDistilledValueReturnedFromJS(kTitle, kContent, image_indices, "");
- EXPECT_CALL(page_factory_,
- CreateDistillerPageMock(_)).WillOnce(
- CreateMockDistillerPage(list.get(), GURL(kURL)));
+ EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
+ .WillOnce(CreateMockDistillerPage(list.get(), GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
distiller_->Init();
- distiller_->DistillPage(
- GURL(kURL),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+ DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
EXPECT_EQ(article_proto_->pages_size(), 1);
@@ -226,59 +348,32 @@ TEST_F(DistillerTest, DistillPageWithImages) {
TEST_F(DistillerTest, DistillMultiplePages) {
base::MessageLoopForUI loop;
- const int kNumPages = 8;
- vector<int> image_indices[kNumPages];
- string content[kNumPages];
- string page_urls[kNumPages];
- scoped_ptr<base::ListValue> list[kNumPages];
+ const size_t kNumPages = 8;
+ scoped_ptr<MultipageDistillerData> distiller_data =
+ CreateMultipageDistillerDataWithoutImages(kNumPages);
+ // Add images.
int next_image_number = 0;
-
- for (int page_num = 0; page_num < kNumPages; ++page_num) {
+ for (size_t page_num = 0; page_num < kNumPages; ++page_num) {
// Each page has different number of images.
- int tot_images = (page_num + kTotalImages) % (kTotalImages + 1);
- for (int img_num = 0; img_num < tot_images; img_num++) {
- image_indices[page_num].push_back(next_image_number);
+ size_t tot_images = (page_num + kTotalImages) % (kTotalImages + 1);
+ vector<int> image_indices;
+ for (size_t img_num = 0; img_num < tot_images; img_num++) {
+ image_indices.push_back(next_image_number);
next_image_number = (next_image_number + 1) % kTotalImages;
}
-
- page_urls[page_num] = "http://a.com/" + base::IntToString(page_num);
- content[page_num] = "Content for page:" + base::IntToString(page_num);
- }
- for (int i = 0; i < kNumPages; ++i) {
- string next_page_url = "";
- if (i + 1 < kNumPages)
- next_page_url = page_urls[i + 1];
-
- list[i] = CreateDistilledValueReturnedFromJS(
- kTitle, content[i], image_indices[i], next_page_url);
+ distiller_data->image_ids.push_back(image_indices);
}
EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
- .WillOnce(CreateMockDistillerPages(list, page_urls, kNumPages, 0));
+ .WillOnce(CreateMockDistillerPages(distiller_data.get(), kNumPages, 0));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
distiller_->Init();
- distiller_->DistillPage(
- GURL(page_urls[0]),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+ DistillPage(distiller_data->page_urls[0]);
base::MessageLoop::current()->RunUntilIdle();
- EXPECT_EQ(kTitle, article_proto_->title());
- EXPECT_EQ(article_proto_->pages_size(), kNumPages);
- for (int page_num = 0; page_num < kNumPages; ++page_num) {
- const DistilledPageProto& page = article_proto_->pages(page_num);
- EXPECT_EQ(content[page_num], page.html());
- EXPECT_EQ(page_urls[page_num], page.url());
- EXPECT_EQ(image_indices[page_num].size(),
- static_cast<size_t>(page.image_size()));
- for (size_t img_num = 0; img_num < image_indices[page_num].size();
- ++img_num) {
- EXPECT_EQ(kImageData[image_indices[page_num][img_num]],
- page.image(img_num).data());
- EXPECT_EQ(GetImageName(page_num + 1, img_num),
- page.image(img_num).name());
- }
- }
+ VerifyArticleProtoMatchesMultipageData(
+ article_proto_.get(), distiller_data.get(), kNumPages);
}
TEST_F(DistillerTest, DistillLinkLoop) {
@@ -291,84 +386,62 @@ TEST_F(DistillerTest, DistillLinkLoop) {
.WillOnce(CreateMockDistillerPage(list.get(), GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
distiller_->Init();
- distiller_->DistillPage(
- GURL(kURL),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+ DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
EXPECT_EQ(article_proto_->pages_size(), 1);
}
-TEST_F(DistillerTest, CheckMaxPageLimit) {
+TEST_F(DistillerTest, CheckMaxPageLimitExtraPage) {
base::MessageLoopForUI loop;
const size_t kMaxPagesInArticle = 10;
- string page_urls[kMaxPagesInArticle];
- scoped_ptr<base::ListValue> list[kMaxPagesInArticle];
+ scoped_ptr<MultipageDistillerData> distiller_data =
+ CreateMultipageDistillerDataWithoutImages(kMaxPagesInArticle);
// Note: Next page url of the last page of article is set. So distiller will
// try to do kMaxPagesInArticle + 1 calls if the max article limit does not
// work.
- string url_prefix = "http://a.com/";
- for (size_t page_num = 0; page_num < kMaxPagesInArticle; ++page_num) {
- page_urls[page_num] = url_prefix + base::IntToString(page_num + 1);
- string content = "Content for page:" + base::IntToString(page_num);
- string next_page_url = url_prefix + base::IntToString(page_num + 2);
- list[page_num] = CreateDistilledValueReturnedFromJS(
- kTitle, content, vector<int>(), next_page_url);
- }
-
- EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
- .WillOnce(CreateMockDistillerPages(
- list, page_urls, static_cast<int>(kMaxPagesInArticle), 0));
-
+ scoped_ptr<base::ListValue> last_page_data =
+ CreateDistilledValueReturnedFromJS(
+ kTitle,
+ distiller_data->content[kMaxPagesInArticle - 1],
+ vector<int>(),
+ "",
+ distiller_data->page_urls[kMaxPagesInArticle - 2]);
+
+ distiller_data->distilled_values.pop_back();
+ distiller_data->distilled_values.push_back(last_page_data.release());
+
+ EXPECT_CALL(page_factory_, CreateDistillerPageMock(_)).WillOnce(
+ CreateMockDistillerPages(distiller_data.get(), kMaxPagesInArticle, 0));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
distiller_->SetMaxNumPagesInArticle(kMaxPagesInArticle);
distiller_->Init();
- distiller_->DistillPage(
- GURL(page_urls[0]),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+ DistillPage(distiller_data->page_urls[0]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
EXPECT_EQ(kMaxPagesInArticle,
static_cast<size_t>(article_proto_->pages_size()));
+}
- // Now check if distilling an article with exactly the page limit works by
- // resetting the next page url of the last page of the article.
- list[kMaxPagesInArticle - 1] =
- CreateDistilledValueReturnedFromJS(kTitle, "Content", vector<int>(), "");
- EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
- .WillOnce(CreateMockDistillerPages(
- list, page_urls, static_cast<int>(kMaxPagesInArticle), 0));
+TEST_F(DistillerTest, CheckMaxPageLimitExactLimit) {
+ base::MessageLoopForUI loop;
+ const size_t kMaxPagesInArticle = 10;
+ scoped_ptr<MultipageDistillerData> distiller_data =
+ CreateMultipageDistillerDataWithoutImages(kMaxPagesInArticle);
+ EXPECT_CALL(page_factory_, CreateDistillerPageMock(_)).WillOnce(
+ CreateMockDistillerPages(distiller_data.get(), kMaxPagesInArticle, 0));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
- distiller_->SetMaxNumPagesInArticle(kMaxPagesInArticle);
- distiller_->Init();
- distiller_->DistillPage(
- GURL(page_urls[0]),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
- base::MessageLoop::current()->RunUntilIdle();
- EXPECT_EQ(kTitle, article_proto_->title());
- EXPECT_EQ(kMaxPagesInArticle,
- static_cast<size_t>(article_proto_->pages_size()));
-
- // Now check if distilling an article with exactly the page limit works by
- // resetting the next page url of the last page of the article.
- list[kMaxPagesInArticle - 1] =
- CreateDistilledValueReturnedFromJS(kTitle, "Content", vector<int>(), "");
- EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
- .WillOnce(CreateMockDistillerPages(
- list, page_urls, static_cast<int>(kMaxPagesInArticle), 0));
-
- distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
+ // Check if distilling an article with exactly the page limit works.
distiller_->SetMaxNumPagesInArticle(kMaxPagesInArticle);
distiller_->Init();
- distiller_->DistillPage(
- GURL(page_urls[0]),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+
+ DistillPage(distiller_data->page_urls[0]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
EXPECT_EQ(kMaxPagesInArticle,
@@ -383,9 +456,7 @@ TEST_F(DistillerTest, SinglePageDistillationFailure) {
.WillOnce(CreateMockDistillerPage(nullValue.get(), GURL(kURL)));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
distiller_->Init();
- distiller_->DistillPage(
- GURL(kURL),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+ DistillPage(kURL);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ("", article_proto_->title());
EXPECT_EQ(0, article_proto_->pages_size());
@@ -393,83 +464,125 @@ TEST_F(DistillerTest, SinglePageDistillationFailure) {
TEST_F(DistillerTest, MultiplePagesDistillationFailure) {
base::MessageLoopForUI loop;
- const int kNumPages = 8;
- string content[kNumPages];
- string page_urls[kNumPages];
- scoped_ptr<base::Value> distilled_values[kNumPages];
- // The page number of the failed page.
- int failed_page_num = 3;
- string url_prefix = "http://a.com/";
- for (int page_num = 0; page_num < kNumPages; ++page_num) {
- page_urls[page_num] = url_prefix + base::IntToString(page_num);
- content[page_num] = "Content for page:" + base::IntToString(page_num);
- string next_page_url = url_prefix + base::IntToString(page_num + 1);
- if (page_num != failed_page_num) {
- distilled_values[page_num] = CreateDistilledValueReturnedFromJS(
- kTitle, content[page_num], vector<int>(), next_page_url);
- } else {
- distilled_values[page_num].reset(base::Value::CreateNullValue());
- }
- }
+ const size_t kNumPages = 8;
+ scoped_ptr<MultipageDistillerData> distiller_data =
+ CreateMultipageDistillerDataWithoutImages(kNumPages);
+ // The page number of the failed page.
+ size_t failed_page_num = 3;
+ // reset distilled data of the failed page.
+ distiller_data->distilled_values.erase(
+ distiller_data->distilled_values.begin() + failed_page_num);
+ distiller_data->distilled_values.insert(
+ distiller_data->distilled_values.begin() + failed_page_num,
+ base::Value::CreateNullValue());
// Expect only calls till the failed page number.
+ EXPECT_CALL(page_factory_, CreateDistillerPageMock(_)).WillOnce(
+ CreateMockDistillerPages(distiller_data.get(), failed_page_num + 1, 0));
+
+ distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
+ distiller_->Init();
+ DistillPage(distiller_data->page_urls[0]);
+ base::MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(kTitle, article_proto_->title());
+ VerifyArticleProtoMatchesMultipageData(
+ article_proto_.get(), distiller_data.get(), failed_page_num);
+}
+
+TEST_F(DistillerTest, DistillPreviousPage) {
+ base::MessageLoopForUI loop;
+ const size_t kNumPages = 8;
+
+ // The page number of the article on which distillation starts.
+ int start_page_num = 3;
+ scoped_ptr<MultipageDistillerData> distiller_data =
+ CreateMultipageDistillerDataWithoutImages(kNumPages);
+
+ EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
+ .WillOnce(CreateMockDistillerPages(
+ distiller_data.get(), kNumPages, start_page_num));
+
+ distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
+ distiller_->Init();
+ DistillPage(distiller_data->page_urls[start_page_num]);
+ base::MessageLoop::current()->RunUntilIdle();
+ VerifyArticleProtoMatchesMultipageData(
+ article_proto_.get(), distiller_data.get(), kNumPages);
+}
+
+TEST_F(DistillerTest, IncrementalUpdates) {
+ base::MessageLoopForUI loop;
+ const size_t kNumPages = 8;
+
+ // The page number of the article on which distillation starts.
+ int start_page_num = 3;
+ scoped_ptr<MultipageDistillerData> distiller_data =
+ CreateMultipageDistillerDataWithoutImages(kNumPages);
+
EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
.WillOnce(CreateMockDistillerPages(
- distilled_values, page_urls, failed_page_num + 1, 0));
+ distiller_data.get(), kNumPages, start_page_num));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
distiller_->Init();
- distiller_->DistillPage(
- GURL(page_urls[0]),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+ DistillPage(distiller_data->page_urls[start_page_num]);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_EQ(kTitle, article_proto_->title());
- EXPECT_EQ(article_proto_->pages_size(), failed_page_num);
- for (int page_num = 0; page_num < failed_page_num; ++page_num) {
- const DistilledPageProto& page = article_proto_->pages(page_num);
- EXPECT_EQ(content[page_num], page.html());
- EXPECT_EQ(page_urls[page_num], page.url());
- }
+ EXPECT_EQ(kNumPages, static_cast<size_t>(article_proto_->pages_size()));
+ EXPECT_EQ(kNumPages, in_sequence_updates_.size());
+
+ VerifyIncrementalUpdatesMatch(
+ distiller_data.get(), kNumPages, in_sequence_updates_, start_page_num);
}
-TEST_F(DistillerTest, DistillPreviousPage) {
+TEST_F(DistillerTest, IncrementalUpdatesDoNotDeleteFinalArticle) {
base::MessageLoopForUI loop;
- const int kNumPages = 8;
- string content[kNumPages];
- string page_urls[kNumPages];
- scoped_ptr<base::Value> distilled_values[kNumPages];
+ const size_t kNumPages = 8;
+ int start_page_num = 3;
+ scoped_ptr<MultipageDistillerData> distiller_data =
+ CreateMultipageDistillerDataWithoutImages(kNumPages);
+
+ EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
+ .WillOnce(CreateMockDistillerPages(
+ distiller_data.get(), kNumPages, start_page_num));
+
+ distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
+ distiller_->Init();
+ DistillPage(distiller_data->page_urls[start_page_num]);
+ base::MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(kNumPages, in_sequence_updates_.size());
+
+ in_sequence_updates_.clear();
+
+ // Should still be able to access article and pages.
+ VerifyArticleProtoMatchesMultipageData(
+ article_proto_.get(), distiller_data.get(), kNumPages);
+}
+TEST_F(DistillerTest, DeletingArticleDoesNotInterfereWithUpdates) {
+ base::MessageLoopForUI loop;
+ const size_t kNumPages = 8;
+ scoped_ptr<MultipageDistillerData> distiller_data =
+ CreateMultipageDistillerDataWithoutImages(kNumPages);
// The page number of the article on which distillation starts.
- int start_page_number = 3;
- string url_prefix = "http://a.com/";
- for (int page_no = 0; page_no < kNumPages; ++page_no) {
- page_urls[page_no] = url_prefix + base::IntToString(page_no);
- content[page_no] = "Content for page:" + base::IntToString(page_no);
- string next_page_url = (page_no + 1 < kNumPages)
- ? url_prefix + base::IntToString(page_no + 1)
- : "";
- string prev_page_url = (page_no > 0) ? page_urls[page_no - 1] : "";
- distilled_values[page_no] = CreateDistilledValueReturnedFromJS(
- kTitle, content[page_no], vector<int>(), next_page_url, prev_page_url);
- }
+ int start_page_num = 3;
EXPECT_CALL(page_factory_, CreateDistillerPageMock(_))
.WillOnce(CreateMockDistillerPages(
- distilled_values, page_urls, kNumPages, start_page_number));
+ distiller_data.get(), kNumPages, start_page_num));
distiller_.reset(new DistillerImpl(page_factory_, url_fetcher_factory_));
distiller_->Init();
- distiller_->DistillPage(
- GURL(page_urls[start_page_number]),
- base::Bind(&DistillerTest::OnDistillPageDone, base::Unretained(this)));
+ DistillPage(distiller_data->page_urls[start_page_num]);
base::MessageLoop::current()->RunUntilIdle();
+ EXPECT_EQ(kNumPages, in_sequence_updates_.size());
EXPECT_EQ(kTitle, article_proto_->title());
- EXPECT_EQ(kNumPages, article_proto_->pages_size());
- for (int page_no = 0; page_no < kNumPages; ++page_no) {
- const DistilledPageProto& page = article_proto_->pages(page_no);
- EXPECT_EQ(content[page_no], page.html());
- EXPECT_EQ(page_urls[page_no], page.url());
- }
+ EXPECT_EQ(kNumPages, static_cast<size_t>(article_proto_->pages_size()));
+
+ // Delete the article.
+ article_proto_.reset();
+ VerifyIncrementalUpdatesMatch(
+ distiller_data.get(), kNumPages, in_sequence_updates_, start_page_num);
}
} // namespace dom_distiller
diff --git a/components/dom_distiller/core/dom_distiller_service_unittest.cc b/components/dom_distiller/core/dom_distiller_service_unittest.cc
index 69e8e3a..72dabef 100644
--- a/components/dom_distiller/core/dom_distiller_service_unittest.cc
+++ b/components/dom_distiller/core/dom_distiller_service_unittest.cc
@@ -33,6 +33,8 @@ class FakeViewRequestDelegate : public ViewRequestDelegate {
public:
virtual ~FakeViewRequestDelegate() {}
MOCK_METHOD1(OnArticleReady, void(const DistilledArticleProto* proto));
+ MOCK_METHOD1(OnArticleUpdated,
+ void(ArticleDistillationUpdate article_update));
};
class MockDistillerObserver : public DomDistillerObserver {
@@ -120,7 +122,7 @@ TEST_F(DomDistillerServiceTest, TestViewEntry) {
scoped_ptr<ViewerHandle> handle =
service_->ViewEntry(&viewer_delegate, entry_id);
- ASSERT_FALSE(distiller->GetCallback().is_null());
+ ASSERT_FALSE(distiller->GetArticleCallback().is_null());
scoped_ptr<DistilledArticleProto> proto = CreateDefaultArticle();
EXPECT_CALL(viewer_delegate, OnArticleReady(proto.get()));
@@ -137,7 +139,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrl) {
GURL url("http://www.example.com/p1");
scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url);
- ASSERT_FALSE(distiller->GetCallback().is_null());
+ ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl());
scoped_ptr<DistilledArticleProto> proto = CreateDefaultArticle();
@@ -162,7 +164,7 @@ TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) {
scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url);
scoped_ptr<ViewerHandle> handle2 = service_->ViewUrl(&viewer_delegate2, url2);
- ASSERT_FALSE(distiller->GetCallback().is_null());
+ ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl());
scoped_ptr<DistilledArticleProto> proto = CreateDefaultArticle();
@@ -170,7 +172,7 @@ TEST_F(DomDistillerServiceTest, TestMultipleViewUrl) {
RunDistillerCallback(distiller, proto.Pass());
- ASSERT_FALSE(distiller2->GetCallback().is_null());
+ ASSERT_FALSE(distiller2->GetArticleCallback().is_null());
EXPECT_EQ(url2, distiller2->GetUrl());
scoped_ptr<DistilledArticleProto> proto2 = CreateDefaultArticle();
@@ -192,7 +194,7 @@ TEST_F(DomDistillerServiceTest, TestViewUrlCancelled) {
GURL url("http://www.example.com/p1");
scoped_ptr<ViewerHandle> handle = service_->ViewUrl(&viewer_delegate, url);
- ASSERT_FALSE(distiller->GetCallback().is_null());
+ ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl());
EXPECT_CALL(viewer_delegate, OnArticleReady(_)).Times(0);
@@ -234,7 +236,7 @@ TEST_F(DomDistillerServiceTest, TestAddAndRemoveEntry) {
std::string entry_id = service_->AddToList(url, ArticleCallback(&article_cb));
- ASSERT_FALSE(distiller->GetCallback().is_null());
+ ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(url, distiller->GetUrl());
scoped_ptr<DistilledArticleProto> proto = CreateArticleWithURL(url.spec());
@@ -389,7 +391,7 @@ TEST_F(DomDistillerServiceTest, TestMultiplePageArticle) {
service_->AddToList(pages_url[0], ArticleCallback(&article_cb));
ArticleEntry entry;
- ASSERT_FALSE(distiller->GetCallback().is_null());
+ ASSERT_FALSE(distiller->GetArticleCallback().is_null());
EXPECT_EQ(pages_url[0], distiller->GetUrl());
// Create the article with pages to pass to the distiller.
diff --git a/components/dom_distiller/core/fake_distiller.cc b/components/dom_distiller/core/fake_distiller.cc
index e6cc979..a37c29a 100644
--- a/components/dom_distiller/core/fake_distiller.cc
+++ b/components/dom_distiller/core/fake_distiller.cc
@@ -21,10 +21,13 @@ FakeDistiller::FakeDistiller(bool execute_callback)
FakeDistiller::~FakeDistiller() { Die(); }
-void FakeDistiller::DistillPage(const GURL& url,
- const DistillerCallback& callback) {
+void FakeDistiller::DistillPage(
+ const GURL& url,
+ const DistillationFinishedCallback& article_callback,
+ const DistillationUpdateCallback& page_callback) {
url_ = url;
- callback_ = callback;
+ article_callback_ = article_callback;
+ page_callback_ = page_callback;
if (execute_callback_) {
scoped_ptr<DistilledArticleProto> proto(new DistilledArticleProto);
proto->add_pages()->set_url(url_.spec());
@@ -43,9 +46,9 @@ void FakeDistiller::RunDistillerCallback(
void FakeDistiller::RunDistillerCallbackInternal(
scoped_ptr<DistilledArticleProto> proto) {
- EXPECT_FALSE(callback_.is_null());
- callback_.Run(proto.Pass());
- callback_.Reset();
+ EXPECT_FALSE(article_callback_.is_null());
+ article_callback_.Run(proto.Pass());
+ article_callback_.Reset();
}
} // namespace test
diff --git a/components/dom_distiller/core/fake_distiller.h b/components/dom_distiller/core/fake_distiller.h
index 98ab709..eeadb62 100644
--- a/components/dom_distiller/core/fake_distiller.h
+++ b/components/dom_distiller/core/fake_distiller.h
@@ -5,6 +5,7 @@
#ifndef COMPONENTS_DOM_DISTILLER_CORE_FAKE_DISTILLER_H_
#define COMPONENTS_DOM_DISTILLER_CORE_FAKE_DISTILLER_H_
+#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/article_entry.h"
#include "components/dom_distiller/core/distiller.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -32,20 +33,25 @@ class FakeDistiller : public Distiller {
MOCK_METHOD0(Die, void());
virtual void DistillPage(const GURL& url,
- const DistillerCallback& callback) OVERRIDE;
+ const DistillationFinishedCallback& article_callback,
+ const DistillationUpdateCallback& page_callback)
+ OVERRIDE;
void RunDistillerCallback(scoped_ptr<DistilledArticleProto> proto);
GURL GetUrl() { return url_; }
- DistillerCallback GetCallback() { return callback_; }
+ DistillationFinishedCallback GetArticleCallback() {
+ return article_callback_;
+ }
private:
void RunDistillerCallbackInternal(scoped_ptr<DistilledArticleProto> proto);
bool execute_callback_;
GURL url_;
- DistillerCallback callback_;
+ DistillationFinishedCallback article_callback_;
+ DistillationUpdateCallback page_callback_;
};
} // namespace test
diff --git a/components/dom_distiller/core/task_tracker.cc b/components/dom_distiller/core/task_tracker.cc
index 1afd482..4d2afd7 100644
--- a/components/dom_distiller/core/task_tracker.cc
+++ b/components/dom_distiller/core/task_tracker.cc
@@ -41,7 +41,9 @@ void TaskTracker::StartDistiller(DistillerFactory* factory) {
distiller_ = factory->CreateDistiller();
distiller_->DistillPage(url,
- base::Bind(&TaskTracker::OnDistilledDataReady,
+ base::Bind(&TaskTracker::OnDistilledArticleReady,
+ weak_ptr_factory_.GetWeakPtr()),
+ base::Bind(&TaskTracker::OnArticleDistillationUpdated,
weak_ptr_factory_.GetWeakPtr()));
}
@@ -139,7 +141,7 @@ void TaskTracker::NotifyViewer(ViewRequestDelegate* delegate) {
delegate->OnArticleReady(distilled_article_.get());
}
-void TaskTracker::OnDistilledDataReady(
+void TaskTracker::OnDistilledArticleReady(
scoped_ptr<DistilledArticleProto> distilled_article) {
distilled_article_ = distilled_article.Pass();
bool distillation_successful = false;
@@ -164,4 +166,11 @@ void TaskTracker::OnDistilledDataReady(
DoSaveCallbacks(distillation_successful);
}
+void TaskTracker::OnArticleDistillationUpdated(
+ const ArticleDistillationUpdate& article_update) {
+ for (size_t i = 0; i < viewers_.size(); ++i) {
+ viewers_[i]->OnArticleUpdated(article_update);
+ }
+}
+
} // namespace dom_distiller
diff --git a/components/dom_distiller/core/task_tracker.h b/components/dom_distiller/core/task_tracker.h
index 3c9effb..42521e6 100644
--- a/components/dom_distiller/core/task_tracker.h
+++ b/components/dom_distiller/core/task_tracker.h
@@ -11,6 +11,7 @@
#include "base/bind.h"
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
+#include "components/dom_distiller/core/article_distillation_update.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"
@@ -44,6 +45,9 @@ class ViewRequestDelegate {
// when the corresponding ViewerHandle is destroyed (or when the
// DomDistillerService is destroyed).
virtual void OnArticleReady(const DistilledArticleProto* article_proto) = 0;
+
+ // Called when an article that is currently under distillation is updated.
+ virtual void OnArticleUpdated(ArticleDistillationUpdate article_update) = 0;
};
// A TaskTracker manages the various tasks related to viewing, saving,
@@ -90,8 +94,10 @@ class TaskTracker {
bool HasUrl(const GURL& url) const;
private:
- void OnDistilledDataReady(
+ void OnDistilledArticleReady(
scoped_ptr<DistilledArticleProto> distilled_article);
+ void OnArticleDistillationUpdated(
+ const ArticleDistillationUpdate& article_update);
// Posts a task to run DoSaveCallbacks with |distillation_succeeded|.
void ScheduleSaveCallbacks(bool distillation_succeeded);
diff --git a/components/dom_distiller/core/task_tracker_unittest.cc b/components/dom_distiller/core/task_tracker_unittest.cc
index c88becb..c6eef4b 100644
--- a/components/dom_distiller/core/task_tracker_unittest.cc
+++ b/components/dom_distiller/core/task_tracker_unittest.cc
@@ -5,6 +5,7 @@
#include "components/dom_distiller/core/task_tracker.h"
#include "base/run_loop.h"
+#include "components/dom_distiller/core/article_distillation_update.h"
#include "components/dom_distiller/core/article_entry.h"
#include "components/dom_distiller/core/fake_distiller.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -20,6 +21,8 @@ class FakeViewRequestDelegate : public ViewRequestDelegate {
virtual ~FakeViewRequestDelegate() {}
MOCK_METHOD1(OnArticleReady,
void(const DistilledArticleProto* article_proto));
+ MOCK_METHOD1(OnArticleUpdated,
+ void(ArticleDistillationUpdate article_update));
};
class TestCancelCallback {