add pagination info to DistilledPageProto

components/dom_distiller/core/distiller_unittest.cc

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 a6ef0197dad65ce4fe3a642195fc206ba09782a4..63b691257dca1ef22c79d50b9760ef79407a6157 100644
--- a/components/dom_distiller/core/distiller_unittest.cc
+++ b/components/dom_distiller/core/distiller_unittest.cc
@@ -137,20 +137,30 @@ void VerifyIncrementalUpdatesMatch(
   }
 }
 
+string GenerateNextPageUrl(size_t page_num, size_t pages_size,
+    string url_prefix) {

[nyquist, 2014/10/24 20:04:02]

How about using const reference to url_prefix here?

[kuan, 2014/10/24 20:57:32]

Done.  of course, too much java recently and forgot abt cc :(

+  return page_num + 1 < pages_size ?
+      url_prefix + base::IntToString(page_num + 1) : "";
+}
+
+string GeneratePrevPageUrl(size_t page_num, size_t pages_size,

[nyquist, 2014/10/24 20:04:02]

It seems like pages_size might not be needed for this function?

[kuan, 2014/10/24 20:57:32]

Done.

+    string url_prefix) {
+  return page_num > 0 ? url_prefix + base::IntToString(page_num - 1) : "";
+}
+
 scoped_ptr<MultipageDistillerData> CreateMultipageDistillerDataWithoutImages(
     size_t pages_size) {
   scoped_ptr<MultipageDistillerData> result(new MultipageDistillerData());
-  string url_prefix = "http://a.com/";
+  string url_prefix = kURL;
   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 next_page_url =
+        GenerateNextPageUrl(page_num, pages_size, url_prefix);
     string prev_page_url =
-        (page_num > 0) ? result->page_urls[page_num - 1] : "";
+        GeneratePrevPageUrl(page_num, pages_size, url_prefix);

[nyquist, 2014/10/24 20:04:02]

If you end up removing pages_size here, maybe move url_prefix to be the first
argument here and above so the code ends up staying just as readable as it is
right now?

[kuan, 2014/10/24 20:57:32]

Done.

     scoped_ptr<base::Value> distilled_value =
         CreateDistilledValueReturnedFromJS(kTitle,
                                            result->content[page_num],
@@ -165,10 +175,13 @@ scoped_ptr<MultipageDistillerData> CreateMultipageDistillerDataWithoutImages(
 void VerifyArticleProtoMatchesMultipageData(
     const dom_distiller::DistilledArticleProto* article_proto,
     const MultipageDistillerData* distiller_data,
-    size_t pages_size) {
-  ASSERT_EQ(pages_size, static_cast<size_t>(article_proto->pages_size()));
+    size_t distilled_pages_size,
+    size_t total_pages_size) {
+  ASSERT_EQ(distilled_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) {
+  string url_prefix = kURL;
+  for (size_t page_num = 0; page_num < distilled_pages_size; ++page_num) {
     const dom_distiller::DistilledPageProto& page =
         article_proto->pages(page_num);
     EXPECT_EQ(distiller_data->content[page_num], page.html());
@@ -182,6 +195,12 @@ void VerifyArticleProtoMatchesMultipageData(
       EXPECT_EQ(GetImageName(page_num + 1, img_num),
                 page.image(img_num).name());
     }
+    string expected_next_page_url =
+        GenerateNextPageUrl(page_num, total_pages_size, url_prefix);
+    string expected_prev_page_url =
+        GeneratePrevPageUrl(page_num, total_pages_size, url_prefix);
+    EXPECT_EQ(expected_next_page_url, page.pagination_info().next_page());
+    EXPECT_EQ(expected_prev_page_url, page.pagination_info().prev_page());
   }
 }
 
@@ -390,7 +409,7 @@ TEST_F(DistillerTest, DistillMultiplePages) {
       CreateMockDistillerPages(distiller_data.get(), kNumPages, 0).Pass());
   base::MessageLoop::current()->RunUntilIdle();
   VerifyArticleProtoMatchesMultipageData(
-      article_proto_.get(), distiller_data.get(), kNumPages);
+      article_proto_.get(), distiller_data.get(), kNumPages, kNumPages);
 }
 
 TEST_F(DistillerTest, DistillLinkLoop) {
@@ -498,7 +517,7 @@ TEST_F(DistillerTest, MultiplePagesDistillationFailure) {
   base::MessageLoop::current()->RunUntilIdle();
   EXPECT_EQ(kTitle, article_proto_->title());
   VerifyArticleProtoMatchesMultipageData(
-      article_proto_.get(), distiller_data.get(), failed_page_num);
+      article_proto_.get(), distiller_data.get(), failed_page_num, kNumPages);
 }
 
 TEST_F(DistillerTest, DistillPreviousPage) {
@@ -517,7 +536,7 @@ TEST_F(DistillerTest, DistillPreviousPage) {
                   distiller_data.get(), kNumPages, start_page_num).Pass());
   base::MessageLoop::current()->RunUntilIdle();
   VerifyArticleProtoMatchesMultipageData(
-      article_proto_.get(), distiller_data.get(), kNumPages);
+      article_proto_.get(), distiller_data.get(), kNumPages, kNumPages);
 }
 
 TEST_F(DistillerTest, IncrementalUpdates) {
@@ -562,7 +581,7 @@ TEST_F(DistillerTest, IncrementalUpdatesDoNotDeleteFinalArticle) {
 
   // Should still be able to access article and pages.
   VerifyArticleProtoMatchesMultipageData(
-      article_proto_.get(), distiller_data.get(), kNumPages);
+      article_proto_.get(), distiller_data.get(), kNumPages, kNumPages);
 }
 
 TEST_F(DistillerTest, DeletingArticleDoesNotInterfereWithUpdates) {