Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(185)

Unified Diff: components/dom_distiller/core/distiller_unittest.cc

Issue 178303004: Add incremental updates for multipage distillation. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Address comments. Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/dom_distiller/core/distiller_unittest.cc
diff --git a/components/dom_distiller/core/distiller_unittest.cc b/components/dom_distiller/core/distiller_unittest.cc
index d3467c72aec481f1117c2b21b06cab57a27898c2..7eda8403f1dd856d07b3cd351537561d60ab7c4c 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"
@@ -58,6 +60,125 @@ namespace {
return list.Pass();
}
+ // Return the sequence in which Distiller will distill pages.
+ // Note: ignores any delays due to fetching images etc.
+ vector<int> get_pages_in_sequence(int start_page_num, int num_pages) {
nyquist 2014/03/03 23:00:12 GetPagesInSequence
+ // 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);
+ };
+
+ 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 =
+ get_pages_in_sequence(start_page_num, num_pages_in_article);
+ // Updates should contain a list of pages. Pages in a update should be in
nyquist 2014/03/03 23:00:12 'an update'
shashi 2014/03/04 19:47:23 Done.
+ // the correct ascending page order regardless of start_page_num.
nyquist 2014/03/03 23:00:12 |start_page_num|
shashi 2014/03/04 19:47:23 Done.
+ // 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)->data.url());
+ EXPECT_EQ(distiller_data->content[actual_page_num],
+ update.GetDistilledPage(j)->data.html());
+ }
+ }
+ }
+
+ 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
namespace dom_distiller {
@@ -85,7 +206,6 @@ class TestDistillerURLFetcher : public DistillerURLFetcher {
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 = get_pages_in_sequence(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

Powered by Google App Engine
This is Rietveld 408576698