Commit a61f420b authored by lazyboy's avatar lazyboy Committed by Commit bot
Browse files

downloads.query: parse numerical query properties as double, not int.

In r244404, we changed these from ints to doubles. However, we still
seem to parse these through Value::GetAsInteger. This always fails.
Change it to use Value::GetAsDouble instead.

BUG=617435
Test=See bug description for repro test case.

Review-Url: https://codereview.chromium.org/2092963002
Cr-Commit-Position: refs/heads/master@{#402332}
parent 18fd8bf0
......@@ -41,8 +41,9 @@ template <typename T> bool GetAs(const base::Value& in, T* out);
template<> bool GetAs(const base::Value& in, bool* out) {
return in.GetAsBoolean(out);
}
template<> bool GetAs(const base::Value& in, int* out) {
return in.GetAsInteger(out);
template <>
bool GetAs(const base::Value& in, double* out) {
return in.GetAsDouble(out);
}
template<> bool GetAs(const base::Value& in, std::string* out) {
return in.GetAsString(out);
......@@ -125,11 +126,11 @@ static DownloadDangerType GetDangerType(const DownloadItem& item) {
return item.GetDangerType();
}
static int GetReceivedBytes(const DownloadItem& item) {
static double GetReceivedBytes(const DownloadItem& item) {
return item.GetReceivedBytes();
}
static int GetTotalBytes(const DownloadItem& item) {
static double GetTotalBytes(const DownloadItem& item) {
return item.GetTotalBytes();
}
......@@ -263,7 +264,7 @@ bool DownloadQuery::AddFilter(DownloadQuery::FilterType type,
const base::Value& value) {
switch (type) {
case FILTER_BYTES_RECEIVED:
return AddFilter(BuildFilter<int>(value, EQ, &GetReceivedBytes));
return AddFilter(BuildFilter<double>(value, EQ, &GetReceivedBytes));
case FILTER_DANGER_ACCEPTED:
return AddFilter(BuildFilter<bool>(value, EQ, &GetDangerAccepted));
case FILTER_EXISTS:
......@@ -295,11 +296,11 @@ bool DownloadQuery::AddFilter(DownloadQuery::FilterType type,
case FILTER_START_TIME:
return AddFilter(BuildFilter<std::string>(value, EQ, &GetStartTime));
case FILTER_TOTAL_BYTES:
return AddFilter(BuildFilter<int>(value, EQ, &GetTotalBytes));
return AddFilter(BuildFilter<double>(value, EQ, &GetTotalBytes));
case FILTER_TOTAL_BYTES_GREATER:
return AddFilter(BuildFilter<int>(value, GT, &GetTotalBytes));
return AddFilter(BuildFilter<double>(value, GT, &GetTotalBytes));
case FILTER_TOTAL_BYTES_LESS:
return AddFilter(BuildFilter<int>(value, LT, &GetTotalBytes));
return AddFilter(BuildFilter<double>(value, LT, &GetTotalBytes));
case FILTER_URL:
return AddFilter(BuildFilter<std::string>(value, EQ, &GetUrl));
case FILTER_URL_REGEX:
......@@ -418,10 +419,10 @@ void DownloadQuery::AddSorter(DownloadQuery::SortType type,
sorters_.push_back(Sorter::Build<std::string>(direction, &GetMimeType));
break;
case SORT_BYTES_RECEIVED:
sorters_.push_back(Sorter::Build<int>(direction, &GetReceivedBytes));
sorters_.push_back(Sorter::Build<double>(direction, &GetReceivedBytes));
break;
case SORT_TOTAL_BYTES:
sorters_.push_back(Sorter::Build<int>(direction, &GetTotalBytes));
sorters_.push_back(Sorter::Build<double>(direction, &GetTotalBytes));
break;
}
}
......
......@@ -52,7 +52,7 @@ class DownloadQuery {
// All times are ISO 8601 strings.
enum FilterType {
FILTER_BYTES_RECEIVED, // int
FILTER_BYTES_RECEIVED, // double
FILTER_DANGER_ACCEPTED, // bool
FILTER_ENDED_AFTER, // string
FILTER_ENDED_BEFORE, // string
......@@ -66,9 +66,9 @@ class DownloadQuery {
FILTER_STARTED_AFTER, // string
FILTER_STARTED_BEFORE, // string
FILTER_START_TIME, // string
FILTER_TOTAL_BYTES, // int
FILTER_TOTAL_BYTES_GREATER, // int
FILTER_TOTAL_BYTES_LESS, // int
FILTER_TOTAL_BYTES, // double
FILTER_TOTAL_BYTES_GREATER, // double
FILTER_TOTAL_BYTES_LESS, // double
FILTER_URL, // string
FILTER_URL_REGEX, // string
};
......
......@@ -38,6 +38,11 @@ static const int kSomeKnownTime = 1355864160;
static const char kSomeKnownTime8601[] = "2012-12-18T20:56:0";
static const char k8601Suffix[] = ".000Z";
static const int64_t kEightGB = 1LL << 33;
static const int64_t kSixteenGB = 1LL << 34;
static const double kEightGBDouble = 8.0 * (1LL << 30);
static const double kNineGBDouble = 9.0 * (1LL << 30);
bool IdNotEqual(uint32_t not_id, const DownloadItem& item) {
return item.GetId() != not_id;
}
......@@ -109,8 +114,9 @@ template<> void DownloadQueryTest::AddFilter(
CHECK(query_.AddFilter(name, *value.get()));
}
template<> void DownloadQueryTest::AddFilter(
DownloadQuery::FilterType name, int cpp_value) {
template <>
void DownloadQueryTest::AddFilter(DownloadQuery::FilterType name,
double cpp_value) {
std::unique_ptr<base::Value> value(new base::FundamentalValue(cpp_value));
CHECK(query_.AddFilter(name, *value.get()));
}
......@@ -329,7 +335,7 @@ TEST_F(DownloadQueryTest, DownloadQueryTest_FilterBytesReceived) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetReceivedBytes()).WillRepeatedly(Return(0));
EXPECT_CALL(mock(1), GetReceivedBytes()).WillRepeatedly(Return(1));
AddFilter(DownloadQuery::FILTER_BYTES_RECEIVED, 0);
AddFilter(DownloadQuery::FILTER_BYTES_RECEIVED, 0.0);
ExpectStandardFilterResults();
}
......@@ -502,27 +508,91 @@ TEST_F(DownloadQueryTest, DownloadQueryTest_SortEndTime) {
ExpectSortInverted();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesGreater) {
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesGreater1) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(2));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(1));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_GREATER, 1);
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_GREATER, 1.0);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesGreater2) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(2));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(1));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_GREATER, 1.2);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesGreater3) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(kSixteenGB));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(kEightGB));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_GREATER, kNineGBDouble);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesLess) {
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesGreater4) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(kSixteenGB));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(kEightGB));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_GREATER, kEightGBDouble + 1.0);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesLess1) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(2));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(4));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_LESS, 4);
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_LESS, 4.0);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesLess2) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(1));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(2));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_LESS, 1.2);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesLess3) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(kEightGB));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(kSixteenGB));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_LESS, kEightGBDouble + 1.0);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytesLess4) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(kEightGB));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(kSixteenGB));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES_LESS, kNineGBDouble);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytes) {
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytes1) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(2));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(4));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES, 2);
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES, 2.0);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytes2) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(1));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(2));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES, 1.0);
ExpectStandardFilterResults();
}
TEST_F(DownloadQueryTest, DownloadQueryTest_FilterTotalBytes3) {
CreateMocks(2);
EXPECT_CALL(mock(0), GetTotalBytes()).WillRepeatedly(Return(kEightGB));
EXPECT_CALL(mock(1), GetTotalBytes()).WillRepeatedly(Return(kSixteenGB));
AddFilter(DownloadQuery::FILTER_TOTAL_BYTES, kEightGBDouble);
ExpectStandardFilterResults();
}
......
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/extensions/api/downloads/downloads_api.h"
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "chrome/browser/download/download_history.h"
#include "chrome/browser/download/download_service_factory.h"
#include "chrome/browser/download/download_service_impl.h"
#include "chrome/browser/extensions/extension_api_unittest.h"
#include "chrome/browser/profiles/profile.h"
#include "content/public/test/mock_download_manager.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using HistoryService = history::HistoryService;
using MockDownloadManager = content::MockDownloadManager;
namespace extensions {
namespace {
// A DownloadService that returns a custom DownloadHistory.
class TestDownloadService : public DownloadServiceImpl {
public:
explicit TestDownloadService(Profile* profile)
: DownloadServiceImpl(profile), profile_(profile) {}
~TestDownloadService() override {}
void set_download_history(std::unique_ptr<DownloadHistory> download_history) {
download_history_.swap(download_history);
}
DownloadHistory* GetDownloadHistory() override {
return download_history_.get();
}
ExtensionDownloadsEventRouter* GetExtensionEventRouter() override {
if (!router_.get()) {
router_.reset(new ExtensionDownloadsEventRouter(
profile_, content::BrowserContext::GetDownloadManager(profile_)));
}
return router_.get();
}
private:
std::unique_ptr<DownloadHistory> download_history_;
std::unique_ptr<ExtensionDownloadsEventRouter> router_;
Profile* profile_;
DISALLOW_COPY_AND_ASSIGN(TestDownloadService);
};
} // namespace
class DownloadsApiUnitTest : public ExtensionApiUnittest {
public:
DownloadsApiUnitTest() {}
~DownloadsApiUnitTest() override {}
void SetUp() override {
ExtensionApiUnittest::SetUp();
manager_.reset(new testing::StrictMock<MockDownloadManager>());
EXPECT_CALL(*manager_, AddObserver(testing::_))
.WillOnce(testing::SaveArg<0>(&download_history_manager_observer_));
EXPECT_CALL(*manager_, RemoveObserver(testing::Eq(testing::ByRef(
download_history_manager_observer_))))
.WillOnce(testing::Assign(
&download_history_manager_observer_,
static_cast<content::DownloadManager::Observer*>(nullptr)));
EXPECT_CALL(*manager_, GetAllDownloads(testing::_))
.Times(testing::AnyNumber());
std::unique_ptr<HistoryAdapter> history_adapter(new HistoryAdapter);
std::unique_ptr<DownloadHistory> download_history(
new DownloadHistory(manager_.get(), std::move(history_adapter)));
TestDownloadService* download_service = static_cast<TestDownloadService*>(
DownloadServiceFactory::GetInstance()->SetTestingFactoryAndUse(
profile(), &TestingDownloadServiceFactory));
ASSERT_TRUE(download_service);
download_service->set_download_history(std::move(download_history));
}
void TearDown() override { ExtensionApiUnittest::TearDown(); }
private:
// A private empty history adapter that does nothing on QueryDownloads().
class HistoryAdapter : public DownloadHistory::HistoryAdapter {
public:
HistoryAdapter() : DownloadHistory::HistoryAdapter(nullptr) {}
private:
void QueryDownloads(
const HistoryService::DownloadQueryCallback& callback) override {}
};
// Constructs and returns a TestDownloadService.
static std::unique_ptr<KeyedService> TestingDownloadServiceFactory(
content::BrowserContext* browser_context);
std::unique_ptr<MockDownloadManager> manager_;
content::DownloadManager::Observer* download_history_manager_observer_;
DISALLOW_COPY_AND_ASSIGN(DownloadsApiUnitTest);
};
// static
std::unique_ptr<KeyedService>
DownloadsApiUnitTest::TestingDownloadServiceFactory(
content::BrowserContext* browser_context) {
return base::WrapUnique(
new TestDownloadService(Profile::FromBrowserContext(browser_context)));
}
// Tests that Number/double properties in query are parsed correctly.
// Regression test for https://crbug.com/617435.
TEST_F(DownloadsApiUnitTest, ParseSearchQuery) {
RunFunction(new DownloadsSearchFunction, "[{\"totalBytesLess\":1}]");
RunFunction(new DownloadsSearchFunction, "[{\"totalBytesGreater\":2}]");
}
} // namespace extensions
......@@ -440,6 +440,7 @@
'browser/extensions/api/dial/dial_device_data_unittest.cc',
'browser/extensions/api/dial/dial_registry_unittest.cc',
'browser/extensions/api/dial/dial_service_unittest.cc',
'browser/extensions/api/downloads/downloads_api_unittest.cc',
'browser/extensions/api/easy_unlock_private/easy_unlock_private_api_chromeos_unittest.cc',
'browser/extensions/api/experience_sampling_private/experience_sampling_private_api_unittest.cc',
'browser/extensions/api/extension_action/browser_action_unittest.cc',
......
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