Add flag "distillability-dev" for distillability development

Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console,
so that it can be accessed by the modeling scripts.

This flag usually go with "--enable-dom-distiller" and
"--reader-mode-heuristics=adaboost".

BUG=610944

[wychen, 2016-05-11 04:57:50]

The CQ bit was checked by wychen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-11 04:58:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1972503002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1972503002/1

[commit-bot: I haz the power, 2016-05-11 05:12:58]

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

[commit-bot: I haz the power, 2016-05-11 05:12:58]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[wychen, 2016-05-11 05:28:56]

Patchset #2 (id:20001) has been deleted

[wychen, 2016-05-11 05:29:13]

The CQ bit was checked by wychen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-11 05:29:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1972503002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1972503002/40001

[wychen, 2016-05-11 05:53:20]

Description was changed from

==========
Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console.

BUG=
==========

to

==========
Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console.

BUG=
==========

[wychen, 2016-05-11 05:53:20]

wychen@chromium.org changed reviewers:
+ esprehn@chromium.org, mdjones@chromium.org

[wychen, 2016-05-11 06:03:47]

Description was changed from

==========
Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console.

BUG=
==========

to

==========
Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console.

BUG=610944
==========

[wychen, 2016-05-11 06:04:24]

Description was changed from

==========
Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console.

BUG=610944
==========

to

==========
Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console,
so that it can be accessed by the modeling scripts.

BUG=610944
==========

[wychen, 2016-05-11 06:04:40]

PTAL

[commit-bot: I haz the power, 2016-05-11 06:30:18]

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

[commit-bot: I haz the power, 2016-05-11 06:30:19]

Dry run: This issue passed the CQ dry run.

[wychen, 2016-05-11 19:41:02]

Description was changed from

==========
Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console,
so that it can be accessed by the modeling scripts.

BUG=610944
==========

to

==========
Add flag "distillability-dev" for distillability development

In Distillability Dev mode, DOM distiller is available for file://
scheme, and the distillability information is dumped to the console,
so that it can be accessed by the modeling scripts.

This flag usually go with "--enable-dom-distiller" and
"--reader-mode-heuristics=adaboost".

BUG=610944
==========

[mdjones, 2016-05-18 19:06:59]

lgtm % question

https://codereview.chromium.org/1972503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/DocumentStatisticsCollector.cpp (right):

https://codereview.chromium.org/1972503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DocumentStatisticsCollector.cpp:273:
ConsoleMessage* consoleMessage = ConsoleMessage::create(ConsoleAPIMessageSource,
DebugMessageLevel, message.toString());
Is this automatically cleaned up (does ConsoleMessage or document own it)?

[wychen, 2016-05-18 19:19:01]

https://codereview.chromium.org/1972503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/DocumentStatisticsCollector.cpp (right):

https://codereview.chromium.org/1972503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DocumentStatisticsCollector.cpp:273:
ConsoleMessage* consoleMessage = ConsoleMessage::create(ConsoleAPIMessageSource,
DebugMessageLevel, message.toString());
On 2016/05/18 19:06:59, mdjones wrote:
> Is this automatically cleaned up (does ConsoleMessage or document own it)?

ConsoleMessage is derived from GarbageCollectedFinalized<ConsoleMessage>, so
it's automatically cleaned up.

[esprehn, 2016-05-23 20:19:42]

https://codereview.chromium.org/1972503002/diff/40001/components/dom_distille...
File components/dom_distiller/content/renderer/distillability_agent.cc (right):

https://codereview.chromium.org/1972503002/diff/40001/components/dom_distille...
components/dom_distiller/content/renderer/distillability_agent.cc:73:
std::stringstream ss;
this is terrible, can we get an argument added to the base::DoubleToString
method?

https://codereview.chromium.org/1972503002/diff/40001/components/dom_distille...
components/dom_distiller/content/renderer/distillability_agent.cc:78: void
dumpDistillability(content::RenderFrame* render_frame,
Dump I think? your name is wrong

https://codereview.chromium.org/1972503002/diff/40001/components/dom_distille...
components/dom_distiller/content/renderer/distillability_agent.cc:97: msg +=
"\"long_page\": " + base::IntToString(long_page) + ",\n";
Can you use base::Value and base::JSONWriter::Write instead? This is pretty hard
to read, and looks error prone

[wychen, 2016-05-23 21:40:24]

https://codereview.chromium.org/1972503002/diff/40001/components/dom_distille...
File components/dom_distiller/content/renderer/distillability_agent.cc (right):

https://codereview.chromium.org/1972503002/diff/40001/components/dom_distille...
components/dom_distiller/content/renderer/distillability_agent.cc:73:
std::stringstream ss;
On 2016/05/23 20:19:42, esprehn wrote:
> this is terrible, can we get an argument added to the base::DoubleToString
> method?

JSON writer takes care of this.

https://codereview.chromium.org/1972503002/diff/40001/components/dom_distille...
components/dom_distiller/content/renderer/distillability_agent.cc:78: void
dumpDistillability(content::RenderFrame* render_frame,
On 2016/05/23 20:19:42, esprehn wrote:
> Dump I think? your name is wrong

Done.

https://codereview.chromium.org/1972503002/diff/40001/components/dom_distille...
components/dom_distiller/content/renderer/distillability_agent.cc:97: msg +=
"\"long_page\": " + base::IntToString(long_page) + ",\n";
On 2016/05/23 20:19:42, esprehn wrote:
> Can you use base::Value and base::JSONWriter::Write instead? This is pretty
hard
> to read, and looks error prone

Awesome idea! Done. The key order is sorted though.

[wychen, 2016-05-27 23:50:09]

wychen@chromium.org changed reviewers:
- esprehn@chromium.org

[wychen, 2016-05-27 23:51:57]

wychen@chromium.org changed reviewers:
+ esprehn@chromium.org

[wychen, 2016-05-27 23:51:58]

Friendly ping esprehn@.

[esprehn, 2016-06-01 05:07:18]

What's the launch timeline for distiller? What team owns this feature long term?
We don't usually add development flags like this.

[wychen, 2016-06-01 05:18:33]

On 2016/06/01 05:07:18, esprehn wrote:
> What's the launch timeline for distiller? What team owns this feature long
term?
> We don't usually add development flags like this.

We plan to be on 1% stable this week, and 100% stable before Q3. We'd like to
further enhance our triggering logic to improve accuracy in the following
milestones, and having a local patch is not very ideal.
Clank team would own this feature long term.

If consolidating all the debug messages to distillability_agent.cc makes more
sense for long-term maintainance, we could do that as well.

[wychen, 2016-06-06 22:54:29]

On 2016/06/01 05:18:33, wychen wrote:
> On 2016/06/01 05:07:18, esprehn wrote:
> > What's the launch timeline for distiller? What team owns this feature long
> term?
> > We don't usually add development flags like this.
> 
> We plan to be on 1% stable this week, and 100% stable before Q3. We'd like to
> further enhance our triggering logic to improve accuracy in the following
> milestones, and having a local patch is not very ideal.
> Clank team would own this feature long term.
> 
> If consolidating all the debug messages to distillability_agent.cc makes more
> sense for long-term maintainance, we could do that as well.

It's all consolidated to distillability_agent.cc.

[wychen, 2016-06-06 22:57:16]

Patchset #5 (id:100001) has been deleted

[wychen, 2016-06-06 23:01:55]

Patchset #5 (id:120001) has been deleted

[wychen, 2016-09-13 01:02:00]

Patchset #8 (id:200001) has been deleted