Write loading signal extra data in JSON

Write loading signal extra data in JSON

Our first attempt wrote siganls in plain text strings.  We think
they will be easier to handle and more extensible if we use JSON
instead.

BUG=706628
Review-Url: https://codereview.chromium.org/2821133003
Cr-Commit-Position: refs/heads/master@{#465359}
Committed: https://chromium.googlesource.com/chromium/src/+/73cbc9649a4cda766fbedb4e27d38872f87f5018

[Pete Williamson, 2017-04-18 00:07:03]

petewil@chromium.org changed reviewers:
+ chili@chromium.org, romax@chromium.org

[Pete Williamson, 2017-04-18 00:31:02]

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

[commit-bot: I haz the power, 2017-04-18 00:31:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[romax, 2017-04-18 00:35:09]

lgtm with nits. and I guess you need someone on the OWNER list :)

https://codereview.chromium.org/2821133003/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/prerendering_loader.cc (right):

https://codereview.chromium.org/2821133003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/prerendering_loader.cc:99: double delay =
delay_so_far.InMilliseconds();
TimeDelta::InMillisecondsF returns a double, while the current one is returning
an int64_t. More like a nit but why we're using double over int64_t?

https://codereview.chromium.org/2821133003/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/prerendering_loader.h (right):

https://codereview.chromium.org/2821133003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/prerendering_loader.h:88: // Returns a
vector of strings for analysis of loading progress.
comments might need to be updated.

[commit-bot: I haz the power, 2017-04-18 01:10:33]

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

[commit-bot: I haz the power, 2017-04-18 01:10:34]

Dry run: This issue passed the CQ dry run.

[Pete Williamson, 2017-04-18 18:26:08]

https://codereview.chromium.org/2821133003/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/prerendering_loader.cc (right):

https://codereview.chromium.org/2821133003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/prerendering_loader.cc:99: double delay =
delay_so_far.InMilliseconds();
On 2017/04/18 00:35:09, romax wrote:
> TimeDelta::InMillisecondsF returns a double, while the current one is
returning
> an int64_t. More like a nit but why we're using double over int64_t?

It turns out that double is an acceptable JSON type, but int64_t is not
(JavaScript doesn't know what to do with it.)  The doc in values.h suggests that
we use double if we need an int64_t.  In this case, especially since we are
subtracting off the times to get a delta in milliseconds, we are not in imminent
danger of exceeding the precision offered by a double, and losing any precision.

https://codereview.chromium.org/2821133003/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/prerendering_loader.h (right):

https://codereview.chromium.org/2821133003/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/prerendering_loader.h:88: // Returns a
vector of strings for analysis of loading progress.
On 2017/04/18 00:35:09, romax wrote:
> comments might need to be updated.

Done.

[chili, 2017-04-18 20:12:39]

lgtm

[Pete Williamson, 2017-04-18 20:13:23]

The CQ bit was checked by petewil@chromium.org

[Pete Williamson, 2017-04-18 20:13:24]

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

[commit-bot: I haz the power, 2017-04-18 20:14:18]

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-04-18 20:51:01]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492546403773680,
"parent_rev": "a5aeed2fbccf03aaaaea0df38fc0ea08ca0f5fbb", "commit_rev":
"73cbc9649a4cda766fbedb4e27d38872f87f5018"}

[commit-bot: I haz the power, 2017-04-18 20:51:14]

Description was changed from

==========
Write loading signal extra data in JSON

Our first attempt wrote siganls in plain text strings.  We think
they will be easier to handle and more extensible if we use JSON
instead.

BUG=706628
==========

to

==========
Write loading signal extra data in JSON

Our first attempt wrote siganls in plain text strings.  We think
they will be easier to handle and more extensible if we use JSON
instead.

BUG=706628

Review-Url: https://codereview.chromium.org/2821133003
Cr-Commit-Position: refs/heads/master@{#465359}
Committed:
https://chromium.googlesource.com/chromium/src/+/73cbc9649a4cda766fbedb4e27d3...
==========

[commit-bot: I haz the power, 2017-04-18 20:51:15]

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