[BlobStorage] Add blob status to blob-internals page

[BlobStorage] Add blob status to blob-internals page

BUG=697310
Review-Url: https://codereview.chromium.org/2723893003
Cr-Commit-Position: refs/heads/master@{#454051}
Committed: https://chromium.googlesource.com/chromium/src/+/a1916107f4e04a80ebd66853739667481c984138

[dmurph, 2017-03-01 03:29:20]

The CQ bit was checked by dmurph@chromium.org

[dmurph, 2017-03-01 03:29:24]

The CQ bit was unchecked by dmurph@chromium.org

[dmurph, 2017-03-01 03:30:00]

dmurph@chromium.org changed reviewers:
+ pwnall@chromium.org

[dmurph, 2017-03-01 03:30:00]

Hey Victor,

I'm updating the blob internals page so I can get better debugging info from bug
reporters. Can you PTAL?

Thanks,
Daniel

[pwnall, 2017-03-01 16:44:05]

LGTM. This is a really good idea! Please see if you can address my comment. If
it's too much work, this should land anyway, because it's an improvement over
the status quo.

https://codereview.chromium.org/2723893003/diff/1/storage/browser/blob/view_b...
File storage/browser/blob/view_blob_internals_job.cc (right):

https://codereview.chromium.org/2723893003/diff/1/storage/browser/blob/view_b...
storage/browser/blob/view_blob_internals_job.cc:71: return "Illegal blob
construction.";
Can you also add the constant names here? It'll make it easier for folks to
search through the code or bug report, and understand what's up.

Possibilities I can think of:
1) "Illegal blob construction. BlobStatus::ERR_INVALID_CONSTRUCTION_ARGUMENTS"
2) "BlobStatus::ERR_INVALID_CONSTRUCTION_ARGUMENTS - Illegal blob construction."

I'm happy with any variation that has both the constant and the explanation, and
makes it easy to copy-paste the constant. A good example is the interstitial
that shows up when you go to https://expired.badssl.com/

[dmurph, 2017-03-01 20:36:35]

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

[dmurph, 2017-03-01 20:36:57]

https://codereview.chromium.org/2723893003/diff/1/storage/browser/blob/view_b...
File storage/browser/blob/view_blob_internals_job.cc (right):

https://codereview.chromium.org/2723893003/diff/1/storage/browser/blob/view_b...
storage/browser/blob/view_blob_internals_job.cc:71: return "Illegal blob
construction.";
On 2017/03/01 16:44:05, pwnall wrote:
> Can you also add the constant names here? It'll make it easier for folks to
> search through the code or bug report, and understand what's up.
> 
> Possibilities I can think of:
> 1) "Illegal blob construction. BlobStatus::ERR_INVALID_CONSTRUCTION_ARGUMENTS"
> 2) "BlobStatus::ERR_INVALID_CONSTRUCTION_ARGUMENTS - Illegal blob
construction."
> 
> I'm happy with any variation that has both the constant and the explanation,
and
> makes it easy to copy-paste the constant. A good example is the interstitial
> that shows up when you go to https://expired.badssl.com/

sgtm - I made it a little shorter by removing the BlobStatus::

[commit-bot: I haz the power, 2017-03-01 20:37:10]

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

[dmurph, 2017-03-01 20:37:14]

The CQ bit was unchecked by dmurph@chromium.org

[dmurph, 2017-03-01 20:37:16]

The CQ bit was checked by dmurph@chromium.org

[dmurph, 2017-03-01 20:37:17]

The patchset sent to the CQ was uploaded after l-g-t-m from pwnall@chromium.org
Link to the patchset: https://codereview.chromium.org/2723893003/#ps20001
(title: "added enum value in string")

[commit-bot: I haz the power, 2017-03-01 20:37:38]

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

[dmurph, 2017-03-01 20:41:39]

The CQ bit was unchecked by dmurph@chromium.org

[dmurph, 2017-03-01 20:44:02]

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

[commit-bot: I haz the power, 2017-03-01 20:44:42]

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

[dmurph, 2017-03-01 21:42:38]

The CQ bit was unchecked by dmurph@chromium.org

[dmurph, 2017-03-01 21:42:42]

The CQ bit was checked by dmurph@chromium.org

[dmurph, 2017-03-01 21:42:43]

The patchset sent to the CQ was uploaded after l-g-t-m from pwnall@chromium.org
Link to the patchset: https://codereview.chromium.org/2723893003/#ps40001
(title: "added BlobStatus::")

[commit-bot: I haz the power, 2017-03-01 21:43:11]

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-03-01 21:51:46]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488404562801130,
"parent_rev": "4dc108a860cc6c2c9c36409a2a6abf6f62bb66c4", "commit_rev":
"09c3af3e38773863ed38ea9f2447fc87510a50b1"}

[commit-bot: I haz the power, 2017-03-01 21:56:26]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488404562801130,
"parent_rev": "d960b4023800e7e90ae6752d2ec2a70bfc991674", "commit_rev":
"a1916107f4e04a80ebd66853739667481c984138"}

[commit-bot: I haz the power, 2017-03-01 21:57:03]

Description was changed from

==========
[BlobStorage] Add blob status to blob-internals page

BUG=697310
==========

to

==========
[BlobStorage] Add blob status to blob-internals page

BUG=697310

Review-Url: https://codereview.chromium.org/2723893003
Cr-Commit-Position: refs/heads/master@{#454051}
Committed:
https://chromium.googlesource.com/chromium/src/+/a1916107f4e04a80ebd668537396...
==========

[commit-bot: I haz the power, 2017-03-01 21:57:04]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/a1916107f4e04a80ebd668537396...