Implement chrome://histograms with network service.

Implement chrome://histograms with network service.

BUG=717714
Review-Url: https://codereview.chromium.org/2874623002
Cr-Commit-Position: refs/heads/master@{#470604}
Committed: https://chromium.googlesource.com/chromium/src/+/17d386e6b5412b5ade8103c66200ced8f0e06e7c

[jam, 2017-05-10 00:39:49]

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

[commit-bot: I haz the power, 2017-05-10 00:41:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jam, 2017-05-10 00:42:02]

jam@chromium.org changed reviewers:
+ yzshen@chromium.org

[commit-bot: I haz the power, 2017-05-10 02:44:00]

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

[commit-bot: I haz the power, 2017-05-10 02:44:01]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has
not started yet; builder either lacks capacity or does not exist (misspelled?))
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet;
builder either lacks capacity or does not exist (misspelled?))
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
build has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[jam, 2017-05-10 03:01:55]

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

[commit-bot: I haz the power, 2017-05-10 03:03:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 04:57:37]

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

[commit-bot: I haz the power, 2017-05-10 04:57:38]

Dry run: This issue passed the CQ dry run.

[yzshen1, 2017-05-10 15:13:43]

LGTM

https://codereview.chromium.org/2874623002/diff/1/content/browser/histogram_i...
File content/browser/histogram_internals_request_job.h (right):

https://codereview.chromium.org/2874623002/diff/1/content/browser/histogram_i...
content/browser/histogram_internals_request_job.h:40: // The string to select
histograms which have |path_| as a substring.
Please update the comment.

https://codereview.chromium.org/2874623002/diff/1/content/browser/histogram_i...
File content/browser/histogram_internals_url_loader.cc (right):

https://codereview.chromium.org/2874623002/diff/1/content/browser/histogram_i...
content/browser/histogram_internals_url_loader.cc:36:
CHECK(mojo::common::BlockingCopyFromString(data, data_pipe.producer_handle));
The data pipe API doesn't guarantee that we could successfully write in one
shot. (Although the current implementation may always succeed for reasonable
size of data.) And we probably don't want any potentially blocking operation on
the UI thread.

It may be fine temporarily, but we (the mojo team) should develop easy-to-use,
work-in-all-case data pipe read/write helper classes so that users don't need to
worry about this.

[jam, 2017-05-10 15:29:58]

https://codereview.chromium.org/2874623002/diff/1/content/browser/histogram_i...
File content/browser/histogram_internals_request_job.h (right):

https://codereview.chromium.org/2874623002/diff/1/content/browser/histogram_i...
content/browser/histogram_internals_request_job.h:40: // The string to select
histograms which have |path_| as a substring.
On 2017/05/10 15:13:43, yzshen1 wrote:
> Please update the comment.

Done.

https://codereview.chromium.org/2874623002/diff/1/content/browser/histogram_i...
File content/browser/histogram_internals_url_loader.cc (right):

https://codereview.chromium.org/2874623002/diff/1/content/browser/histogram_i...
content/browser/histogram_internals_url_loader.cc:36:
CHECK(mojo::common::BlockingCopyFromString(data, data_pipe.producer_handle));
On 2017/05/10 15:13:43, yzshen1 wrote:
> The data pipe API doesn't guarantee that we could successfully write in one
> shot. (Although the current implementation may always succeed for reasonable
> size of data.) And we probably don't want any potentially blocking operation
on
> the UI thread.

Re blocking, I think it's fine for testing pages like this. 
> 
> It may be fine temporarily, but we (the mojo team) should develop easy-to-use,
> work-in-all-case data pipe read/write helper classes so that users don't need
to
> worry about this.

Agree, seems like this method should be it?

[jam, 2017-05-10 15:30:13]

The CQ bit was checked by jam@chromium.org

[jam, 2017-05-10 15:30:13]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2874623002/#ps20001
(title: "add comment")

[commit-bot: I haz the power, 2017-05-10 15:30:48]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 16:40:07]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494430213006660,
"parent_rev": "ed15807589e8d79691b1e9e5673f1d8b9b3a7fa0", "commit_rev":
"17d386e6b5412b5ade8103c66200ced8f0e06e7c"}

[commit-bot: I haz the power, 2017-05-10 16:40:16]

Description was changed from

==========
Implement chrome://histograms with network service.

BUG=717714
==========

to

==========
Implement chrome://histograms with network service.

BUG=717714

Review-Url: https://codereview.chromium.org/2874623002
Cr-Commit-Position: refs/heads/master@{#470604}
Committed:
https://chromium.googlesource.com/chromium/src/+/17d386e6b5412b5ade8103c66200...
==========

[commit-bot: I haz the power, 2017-05-10 16:40:20]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/17d386e6b5412b5ade8103c66200...