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

Issue 608583003: Roll dom_distiller_js and add UMA for word count for distilled pages. (Closed)

Created:
6 years, 2 months ago by nyquist
Modified:
6 years, 2 months ago
Reviewers:
Yaron, Ilya Sherman
CC:
chromium-reviews, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Roll dom_distiller_js and add UMA for word count for distilled pages. For every successfully distilled article a word count is submitted to UMA. The histogram ranges from 1->4000 words with 50 buckets. Changes rolled in from the DOM Distiller repo: bbf7c01 Add StatisticsInfo to DomDistillerResult proto for number of words. fc1a5c1 treat non-breaking space as whitespace de38c78 Expand usage of SimilarSiblingContentExpansion 444a55e reorder table tests 5ff895b add and fix missing tests to suite 970a419 Add SimilarSiblingContentExpansion da76b1e add new table classification heuristic BUG=417049 Committed: https://crrev.com/940e7511e50d7afbe62a1df985bec4d07739c73a Cr-Commit-Position: refs/heads/master@{#296986}

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed comments from isherman and rolled dom distiller #

Unified diffs Side-by-side diffs Delta from patch set Stats (+619 lines, -530 lines) Patch
M components/dom_distiller/core/distiller_page.cc View 1 chunk +40 lines, -28 lines 0 comments Download
M third_party/dom_distiller_js/README.chromium View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/dom_distiller_js/package/js/domdistiller.js View 1 1 chunk +524 lines, -501 lines 0 comments Download
M third_party/dom_distiller_js/package/proto/dom_distiller.proto View 1 2 chunks +5 lines, -0 lines 0 comments Download
M third_party/dom_distiller_js/package/proto_gen/third_party/dom_distiller_js/dom_distiller_json_converter.h View 1 3 chunks +41 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (3 generated)
nyquist
yfriedman: PTAL
6 years, 2 months ago (2014-09-25 18:47:01 UTC) #2
nyquist
isherman: PTAL tools/metrics/histograms/histograms.xml
6 years, 2 months ago (2014-09-25 21:09:50 UTC) #4
Ilya Sherman
LGTM % nits. https://codereview.chromium.org/608583003/diff/1/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/608583003/diff/1/tools/metrics/histograms/histograms.xml#newcode5116 tools/metrics/histograms/histograms.xml:5116: +<histogram name="DomDistiller.Statistics.WordCount"> Optional nit: Might be ...
6 years, 2 months ago (2014-09-25 22:04:01 UTC) #5
Yaron
lgtm but this needs a dom_distiller_js roll
6 years, 2 months ago (2014-09-26 00:35:22 UTC) #6
nyquist
https://codereview.chromium.org/608583003/diff/1/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/608583003/diff/1/tools/metrics/histograms/histograms.xml#newcode5116 tools/metrics/histograms/histograms.xml:5116: +<histogram name="DomDistiller.Statistics.WordCount"> On 2014/09/25 22:04:01, Ilya Sherman wrote: > ...
6 years, 2 months ago (2014-09-26 17:26:38 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/608583003/20001
6 years, 2 months ago (2014-09-26 17:27:12 UTC) #9
commit-bot: I haz the power
Committed patchset #2 (id:20001) as 03119e468d2e2ad66a2b2f60394f2cb8b3cbf2d5
6 years, 2 months ago (2014-09-26 18:34:28 UTC) #10
commit-bot: I haz the power
6 years, 2 months ago (2014-09-26 18:35:04 UTC) #11
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/940e7511e50d7afbe62a1df985bec4d07739c73a
Cr-Commit-Position: refs/heads/master@{#296986}

Powered by Google App Engine
This is Rietveld 408576698