Add more mac executable extensions

Add more mac executable extensions

BUG=600613
Committed: https://crrev.com/cd719f651b57e7235f30b974e763e569f2cc9aeb
Cr-Commit-Position: refs/heads/master@{#387995}

[Jialiu Lin, 2016-04-12 20:44:33]

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

[Jialiu Lin, 2016-04-12 20:46:42]

Hi nparker@ and asvitkine@,
This CL adds 12 mac executable extensions to download protection coverage. PTAL.
Thanks!

[Alexei Svitkine (slow), 2016-04-12 20:50:40]

lgtm

[Nathan Parker, 2016-04-12 20:57:46]

nparker@chromium.org changed reviewers:
+ rsesek@chromium.org

[Nathan Parker, 2016-04-12 20:57:47]

You also need to update download_extensions.cc.  While you're there, add .dmg
since it's missing.

https://codereview.chromium.org/1881763002/diff/1/chrome/common/safe_browsing...
File chrome/common/safe_browsing/download_protection_util.cc (right):

https://codereview.chromium.org/1881763002/diff/1/chrome/common/safe_browsing...
chrome/common/safe_browsing/download_protection_util.cc:501:
{FILE_PATH_LITERAL(".sparseimage"), EXTENSION_SPARSEIMAGE, true, false},
Which if all of these should be marked as an archive?

https://codereview.chromium.org/1881763002/diff/1/chrome/common/safe_browsing...
chrome/common/safe_browsing/download_protection_util.cc:621: else if
(file.MatchesExtension(FILE_PATH_LITERAL(".dmg")) ||
To answer my previous question, I guess since DMGs are called "mac executables"
rather than "archives" here, we could call the rest of these the same.

[Jialiu Lin, 2016-04-12 21:41:14]

Thanks! Both download_extensions.cc and download_protection_service.cc updated.

https://codereview.chromium.org/1881763002/diff/1/chrome/common/safe_browsing...
File chrome/common/safe_browsing/download_protection_util.cc (right):

https://codereview.chromium.org/1881763002/diff/1/chrome/common/safe_browsing...
chrome/common/safe_browsing/download_protection_util.cc:501:
{FILE_PATH_LITERAL(".sparseimage"), EXTENSION_SPARSEIMAGE, true, false},
On 2016/04/12 at 20:57:47, Nathan Parker wrote:
> Which if all of these should be marked as an archive?

They are all counted as executables for now.

https://codereview.chromium.org/1881763002/diff/1/chrome/common/safe_browsing...
chrome/common/safe_browsing/download_protection_util.cc:621: else if
(file.MatchesExtension(FILE_PATH_LITERAL(".dmg")) ||
On 2016/04/12 at 20:57:47, Nathan Parker wrote:
> To answer my previous question, I guess since DMGs are called "mac
executables" rather than "archives" here, we could call the rest of these the
same.

yes, that's my understanding too.

[Jialiu Lin, 2016-04-15 18:02:16]

Hi nparker@ and rsesek@, 
Let me know if you have further comments for this CL.  Thanks!

[Robert Sesek, 2016-04-15 21:21:38]

Sorry, didn't know you were waiting on me.

https://codereview.chromium.org/1881763002/diff/40001/chrome/browser/download...
File chrome/browser/download/download_extensions.cc (right):

https://codereview.chromium.org/1881763002/diff/40001/chrome/browser/download...
chrome/browser/download/download_extensions.cc:504: #if defined(OS_MACOSX) ||
defined(OS_LINUX)
Not sure these apply to OS_LINUX.

[Jialiu Lin, 2016-04-15 21:58:59]

Patchset #4 (id:60001) has been deleted

[Jialiu Lin, 2016-04-15 22:01:06]

Thanks, rsesek@! 

ping nparker@ for owner approval. Thanks!

https://codereview.chromium.org/1881763002/diff/40001/chrome/browser/download...
File chrome/browser/download/download_extensions.cc (right):

https://codereview.chromium.org/1881763002/diff/40001/chrome/browser/download...
chrome/browser/download/download_extensions.cc:504: #if defined(OS_MACOSX) ||
defined(OS_LINUX)
On 2016/04/15 at 21:21:38, Robert Sesek wrote:
> Not sure these apply to OS_LINUX.
You're totally right.... Thanks for catching this. I guess I wasn't thinking...

[Robert Sesek, 2016-04-15 22:01:29]

lgtm

[Nathan Parker, 2016-04-15 23:02:00]

lgtm

[Jialiu Lin, 2016-04-15 23:03:21]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2016-04-15 23:03:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1881763002/#ps80001
(title: "fix mac os ifdef")

[commit-bot: I haz the power, 2016-04-15 23:03:57]

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

[commit-bot: I haz the power, 2016-04-15 23:17:49]

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

[commit-bot: I haz the power, 2016-04-15 23:17:50]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Jialiu Lin, 2016-04-15 23:23:00]

jialiul@chromium.org changed reviewers:
+ asanka@chromium.org

[Jialiu Lin, 2016-04-15 23:23:53]

asanka@, could you take a look at this CL? Need your owner approval for
download_extensions.cc

Thanks!

[asanka, 2016-04-16 03:58:35]

You also need to update download_stats.cc to include the new file types in
kDangerousFileTypes and update DownloadItem.DangerousFileType enum in
histograms.xml. LGTM with that change.

[Jialiu Lin, 2016-04-18 19:31:46]

Thanks, asanka@. download_stats and DangerousFileType histogram enum updated.

[Jialiu Lin, 2016-04-18 20:02:48]

The CQ bit was checked by jialiul@chromium.org

[Jialiu Lin, 2016-04-18 20:02:48]

The patchset sent to the CQ was uploaded after l-g-t-m from asanka@chromium.org,
nparker@chromium.org, asvitkine@chromium.org, rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/1881763002/#ps100001
(title: "update download stats")

[commit-bot: I haz the power, 2016-04-18 20:03:10]

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

[commit-bot: I haz the power, 2016-04-18 20:08:31]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-04-18 20:10:20]

Description was changed from

==========
Add more mac executable extensions

BUG=600613
==========

to

==========
Add more mac executable extensions

BUG=600613

Committed: https://crrev.com/cd719f651b57e7235f30b974e763e569f2cc9aeb
Cr-Commit-Position: refs/heads/master@{#387995}
==========

[commit-bot: I haz the power, 2016-04-18 20:10:22]

Patchset 5 (id:??) landed as
https://crrev.com/cd719f651b57e7235f30b974e763e569f2cc9aeb
Cr-Commit-Position: refs/heads/master@{#387995}