Make file type MPKG generate a download ping

Make file type MPKG generate a download ping

BUG=600908
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/72b3586479fea68f62a871caba880f609c0dbd79
Cr-Commit-Position: refs/heads/master@{#396971}

[Nathan Parker, 2016-05-27 23:44:07]

Description was changed from

==========
Make file type MPKG generate a download ping

BUG=600908
==========

to

==========
Make file type MPKG generate a download ping

BUG=600908
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Nathan Parker, 2016-05-27 23:47:53]

nparker@chromium.org changed reviewers:
+ holte@chromium.org, jialiul@chromium.org

[Nathan Parker, 2016-05-27 23:47:53]

holte: Please review 1-line change to histograms.xml

jialiul: Please review change to download_file_types.asciipb
The docs for that file format are here:
https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/resour...

[Jialiu Lin, 2016-05-27 23:51:31]

LGTM

[vakh (use Gerrit instead), 2016-05-27 23:59:53]

vakh@chromium.org changed reviewers:
+ vakh@chromium.org

[vakh (use Gerrit instead), 2016-05-27 23:59:54]

lgtm

https://codereview.chromium.org/2010333004/diff/1/chrome/browser/resources/sa...
File chrome/browser/resources/safe_browsing/download_file_types.asciipb (right):

https://codereview.chromium.org/2010333004/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/download_file_types.asciipb:2302: # An
alias for PKG files, opened by the Mac installer. http://crbug.com/600908
Not exactly an alias. A super-set, loosely speaking.
See: http://stackoverflow.com/a/1375727

https://codereview.chromium.org/2010333004/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/download_file_types.asciipb:2304:
uma_value: 263
Out of scope for this CL, but:

Do you think it would be useful to somehow specify that this file is somewhat
similar to a PKG file, in a "code" way instead of a comment.
The reason for that is: If tomorrow we decide to change the behavior of PKG like
files, it would be good to make that change in one place and have all related
files reflect it.

https://codereview.chromium.org/2010333004/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/download_file_types.asciipb:2331: # Also
used by Mac installer
To a new reader, it may not clear what "also" means here. Just "Used by Mac
installer" may be sufficient.

[Nathan Parker, 2016-05-28 00:12:43]

https://codereview.chromium.org/2010333004/diff/1/chrome/browser/resources/sa...
File chrome/browser/resources/safe_browsing/download_file_types.asciipb (right):

https://codereview.chromium.org/2010333004/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/download_file_types.asciipb:2304:
uma_value: 263
On 2016/05/27 23:59:54, vakh wrote:
> Out of scope for this CL, but:
> 
> Do you think it would be useful to somehow specify that this file is somewhat
> similar to a PKG file, in a "code" way instead of a comment.
> The reason for that is: If tomorrow we decide to change the behavior of PKG
like
> files, it would be good to make that change in one place and have all related
> files reflect it.

That could be useful.  There are a number of alias-ing/templating/grouping
schemes I had considered for this file which would give it more structure, but
they all made it harder to read.  I think we should see how we use this for a
while and then consider if we need to add more structure.

https://codereview.chromium.org/2010333004/diff/1/chrome/browser/resources/sa...
chrome/browser/resources/safe_browsing/download_file_types.asciipb:2331: # Also
used by Mac installer
On 2016/05/27 23:59:54, vakh wrote:
> To a new reader, it may not clear what "also" means here. Just "Used by Mac
> installer" may be sufficient.

Fixed.  "also" was in reference to the "Linux-specific files" section header,
but that's not obvious.

[Nathan Parker, 2016-05-31 20:16:15]

Friendly ping to holte@.  A one-line review. :-)

[Steven Holte, 2016-05-31 20:31:07]

lgtm

[Nathan Parker, 2016-05-31 21:41:50]

Thanks.  BTW, I also just fixed a histogram that was missing its enum type. I'm
assuming that one is ok.

[Nathan Parker, 2016-05-31 21:41:56]

The CQ bit was checked by nparker@chromium.org

[Nathan Parker, 2016-05-31 21:41:56]

The patchset sent to the CQ was uploaded after l-g-t-m from vakh@chromium.org,
jialiul@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2010333004/#ps60001
(title: "Also fix a SBClientDownload histogram type")

[commit-bot: I haz the power, 2016-05-31 21:42:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2010333004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2010333004/60001

[commit-bot: I haz the power, 2016-05-31 23:23:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-31 23:24:55]

Description was changed from

==========
Make file type MPKG generate a download ping

BUG=600908
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Make file type MPKG generate a download ping

BUG=600908
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/72b3586479fea68f62a871caba880f609c0dbd79
Cr-Commit-Position: refs/heads/master@{#396971}
==========

[commit-bot: I haz the power, 2016-05-31 23:24:56]

Patchset 4 (id:??) landed as
https://crrev.com/72b3586479fea68f62a871caba880f609c0dbd79
Cr-Commit-Position: refs/heads/master@{#396971}