add pagination info to DistilledPageProto

add pagination info to DistilledPageProto

- this is part of the implementation of scoring mechanism for next/prev page
  links.
- pagination info will be passed to the scoring framework.
- standalone content extractor outputs the page links.
- modified tests to verify pagination info.

BUG=425952
Committed: https://crrev.com/42a54f5f7d5da2ea5ccc1d0ffda4a18ea2314ae8
Cr-Commit-Position: refs/heads/master@{#301527}

[kuan, 2014-10-23 22:02:42]

kuan@chromium.org changed reviewers:
+ nyquist@chromium.org

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

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
File components/dom_distiller/core/distiller.cc (right):

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/distiller.cc:183:
page_data->distilled_page_proto->data.mutable_pagination_info()->
Since you added canonical_page to the DistilledPage::PaginationInfo proto, any
specific reason you do not copy the value over from the distiller result here?

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
File components/dom_distiller/core/distiller_unittest.cc (right):

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/distiller_unittest.cc:141: string url_prefix) {
How about using const reference to url_prefix here?

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/distiller_unittest.cc:146: string
GeneratePrevPageUrl(size_t page_num, size_t pages_size,
It seems like pages_size might not be needed for this function?

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/distiller_unittest.cc:163:
GeneratePrevPageUrl(page_num, pages_size, url_prefix);
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]

i've addressed all comments in patch set 2.  ptal.  thx.

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
File components/dom_distiller/core/distiller.cc (right):

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/distiller.cc:183:
page_data->distilled_page_proto->data.mutable_pagination_info()->
On 2014/10/24 20:04:02, nyquist wrote:
> Since you added canonical_page to the DistilledPage::PaginationInfo proto, any
> specific reason you do not copy the value over from the distiller result here?

Done.  no reason, except that it's not used, but i guess the same argument cld
be made for prev_page.

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
File components/dom_distiller/core/distiller_unittest.cc (right):

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/distiller_unittest.cc:141: string url_prefix) {
On 2014/10/24 20:04:02, nyquist wrote:
> How about using const reference to url_prefix here?

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

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/distiller_unittest.cc:146: string
GeneratePrevPageUrl(size_t page_num, size_t pages_size,
On 2014/10/24 20:04:02, nyquist wrote:
> It seems like pages_size might not be needed for this function?

Done.

https://codereview.chromium.org/678543002/diff/1/components/dom_distiller/cor...
components/dom_distiller/core/distiller_unittest.cc:163:
GeneratePrevPageUrl(page_num, pages_size, url_prefix);
On 2014/10/24 20:04:02, nyquist wrote:
> 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?

Done.

[nyquist, 2014-10-27 23:00:34]

lgtm

[kuan, 2014-10-27 23:01:43]

The CQ bit was checked by kuan@chromium.org

[commit-bot: I haz the power, 2014-10-27 23:05:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/678543002/20001

[commit-bot: I haz the power, 2014-10-27 23:19:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-27 23:19:07]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/74272)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/79318)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_32 on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[kuan, 2014-10-27 23:37:55]

The CQ bit was checked by kuan@chromium.org

[commit-bot: I haz the power, 2014-10-27 23:42:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/678543002/40001

[commit-bot: I haz the power, 2014-10-28 01:06:48]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-28 01:07:15]

Patchset 3 (id:??) landed as
https://crrev.com/42a54f5f7d5da2ea5ccc1d0ffda4a18ea2314ae8
Cr-Commit-Position: refs/heads/master@{#301527}