Commit a0d3f678 authored by Matt Mueller's avatar Matt Mueller

Revert "Safebrowsing: allow sending enhanced download protection pings on OSX."

unit_tests failing on bots even though they passed the CQ.

This reverts commit d117e910.

BUG=413967
TBR=noelutz@chromium.org

Review URL: https://codereview.chromium.org/555373005

Cr-Commit-Position: refs/heads/master@{#295213}
parent 6b571de0
......@@ -9,7 +9,6 @@
#include "base/format_macros.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/metrics/field_trial.h"
#include "base/metrics/histogram.h"
#include "base/metrics/sparse_histogram.h"
#include "base/sequenced_task_runner_helpers.h"
......@@ -616,15 +615,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
// Currently, the UI only works on Windows so we don't even bother with
// pinging the server if we're not on Windows.
// TODO(noelutz): change this code once the UI is done for Linux and Mac.
#if defined(OS_MACOSX)
// TODO(mattm): remove this (see crbug.com/414834).
if (base::FieldTrialList::FindFullName("SafeBrowsingOSXClientDownloadPings")
!= "Enabled") {
PostFinishTask(UNKNOWN, REASON_OS_NOT_SUPPORTED);
return;
}
#endif
#if defined(OS_WIN) || defined(OS_MACOSX)
#if defined(OS_WIN)
// The URLFetcher is owned by the UI thread, so post a message to
// start the pingback.
BrowserThread::PostTask(
......@@ -789,13 +780,6 @@ class DownloadProtectionService::CheckClientDownloadRequest
UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckDownloadStats",
reason,
REASON_MAX);
#if defined(OS_MACOSX)
// OSX is currently sending pings only for evaluation purposes, ignore
// the result for now.
// TODO(mattm): remove this and update the ifdef in
// DownloadItemImpl::IsDangerous (see crbug.com/413968).
result = UNKNOWN;
#endif
callback_.Run(result);
item_->RemoveObserver(this);
item_ = NULL;
......@@ -945,17 +929,18 @@ void DownloadProtectionService::CheckDownloadUrl(
bool DownloadProtectionService::IsSupportedDownload(
const content::DownloadItem& item,
const base::FilePath& target_path) const {
// Currently, the UI is only enabled on Windows. On Mac we send the ping but
// ignore the result (see ifdef in FinishRequest). On Linux we still
// Currently, the UI only works on Windows. On Linux and Mac we still
// want to show the dangerous file type warning if the file is possibly
// dangerous which means we have to always return false here.
#if defined(OS_WIN)
DownloadCheckResultReason reason = REASON_MAX;
ClientDownloadRequest::DownloadType type =
ClientDownloadRequest::WIN_EXECUTABLE;
return (CheckClientDownloadRequest::IsSupportedDownload(
item, target_path, &reason, &type) &&
(ClientDownloadRequest::CHROME_EXTENSION != type));
return (CheckClientDownloadRequest::IsSupportedDownload(item, target_path,
&reason, &type) &&
(ClientDownloadRequest::ANDROID_APK == type ||
ClientDownloadRequest::WIN_EXECUTABLE == type ||
ClientDownloadRequest::ZIPPED_EXECUTABLE == type));
#else
return false;
#endif
......
......@@ -510,11 +510,7 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadWhitelistedUrl) {
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this)));
MessageLoop::current()->Run();
#if defined(OS_MACOSX)
EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
#else
EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
#endif
}
TEST_F(DownloadProtectionServiceTest, CheckClientDownloadFetchFailed) {
......@@ -985,7 +981,7 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadValidateRequest) {
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this)));
#if !defined(OS_WIN) && !defined(OS_MACOSX)
#if !defined(OS_WIN)
// SendRequest is not called. Wait for FinishRequest to call our callback.
MessageLoop::current()->Run();
net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
......@@ -1070,7 +1066,7 @@ TEST_F(DownloadProtectionServiceTest,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this)));
#if !defined(OS_WIN) && !defined(OS_MACOSX)
#if !defined(OS_WIN)
// SendRequest is not called. Wait for FinishRequest to call our callback.
MessageLoop::current()->Run();
net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
......@@ -1158,7 +1154,7 @@ TEST_F(DownloadProtectionServiceTest,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this)));
#if !defined(OS_WIN) && !defined(OS_MACOSX)
#if !defined(OS_WIN)
// SendRequest is not called. Wait for FinishRequest to call our callback.
MessageLoop::current()->Run();
net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
......@@ -1233,7 +1229,7 @@ TEST_F(DownloadProtectionServiceTest,
&item,
base::Bind(&DownloadProtectionServiceTest::CheckDoneCallback,
base::Unretained(this)));
#if !defined(OS_WIN) && !defined(OS_MACOSX)
#if !defined(OS_WIN)
// SendRequest is not called. Wait for FinishRequest to call our callback.
MessageLoop::current()->Run();
net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
......
......@@ -215,9 +215,8 @@ message ClientDownloadRequest {
WIN_EXECUTABLE = 0; // Currently all .exe, .cab and .msi files.
CHROME_EXTENSION = 1; // .crx files.
ANDROID_APK = 2; // .apk files.
// .zip files containing one of the other executable types.
// .zip files containing one of the above executable types.
ZIPPED_EXECUTABLE = 3;
MAC_EXECUTABLE = 4; // .dmg, .pkg, etc.
}
optional DownloadType download_type = 10 [default = WIN_EXECUTABLE];
......
......@@ -11,7 +11,6 @@ namespace safe_browsing {
namespace download_protection_util {
bool IsArchiveFile(const base::FilePath& file) {
// TODO(mattm): should .dmg be checked here instead of IsBinaryFile?
return file.MatchesExtension(FILE_PATH_LITERAL(".zip"));
}
......@@ -34,11 +33,6 @@ bool IsBinaryFile(const base::FilePath& file) {
// Chrome extensions and android APKs are also reported.
file.MatchesExtension(FILE_PATH_LITERAL(".crx")) ||
file.MatchesExtension(FILE_PATH_LITERAL(".apk")) ||
// Mac extensions.
file.MatchesExtension(FILE_PATH_LITERAL(".dmg")) ||
file.MatchesExtension(FILE_PATH_LITERAL(".pkg")) ||
file.MatchesExtension(FILE_PATH_LITERAL(".osx")) ||
file.MatchesExtension(FILE_PATH_LITERAL(".app")) ||
// Archives _may_ contain binaries, we'll check in ExtractFileFeatures.
IsArchiveFile(file));
}
......@@ -54,11 +48,6 @@ ClientDownloadRequest::DownloadType GetDownloadType(
// the pingback if we find an executable inside the zip archive.
else if (file.MatchesExtension(FILE_PATH_LITERAL(".zip")))
return ClientDownloadRequest::ZIPPED_EXECUTABLE;
else if (file.MatchesExtension(FILE_PATH_LITERAL(".dmg")) ||
file.MatchesExtension(FILE_PATH_LITERAL(".pkg")) ||
file.MatchesExtension(FILE_PATH_LITERAL(".osx")) ||
file.MatchesExtension(FILE_PATH_LITERAL(".app")))
return ClientDownloadRequest::MAC_EXECUTABLE;
return ClientDownloadRequest::WIN_EXECUTABLE;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment