Commit d3aad5ee authored by Maggie Cai's avatar Maggie Cai Committed by Commit Bot

Settings: Fix data overflow issues for All Sites Page.

Currently the storage data is send to and from the web ui using int,
however when the size gets over 2GB, it will overflow and crash the
browser. This CL convert the data into double and adds unit test for
large storage data.

BUG=960726,943735

Change-Id: I085410fdd3da1674a0b44d1a445927b015219f47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1602234Reviewed-by: default avatarJoshua Bell <jsbell@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658359}
parent 8c433660
...@@ -44,7 +44,7 @@ void MockBrowsingDataLocalStorageHelper::AddLocalStorageSamples() { ...@@ -44,7 +44,7 @@ void MockBrowsingDataLocalStorageHelper::AddLocalStorageSamples() {
void MockBrowsingDataLocalStorageHelper::AddLocalStorageForOrigin( void MockBrowsingDataLocalStorageHelper::AddLocalStorageForOrigin(
const url::Origin& origin, const url::Origin& origin,
size_t size) { int64_t size) {
response_.emplace_back(origin, size, base::Time()); response_.emplace_back(origin, size, base::Time());
origins_[origin] = true; origins_[origin] = true;
} }
......
...@@ -29,7 +29,7 @@ class MockBrowsingDataLocalStorageHelper ...@@ -29,7 +29,7 @@ class MockBrowsingDataLocalStorageHelper
void AddLocalStorageSamples(); void AddLocalStorageSamples();
// Add a LocalStorageInfo entry for a single origin. // Add a LocalStorageInfo entry for a single origin.
void AddLocalStorageForOrigin(const url::Origin& origin, size_t size); void AddLocalStorageForOrigin(const url::Origin& origin, int64_t size);
// Notifies the callback. // Notifies the callback.
void Notify(); void Notify();
......
...@@ -210,8 +210,8 @@ void CreateOrAppendSiteGroupEntry( ...@@ -210,8 +210,8 @@ void CreateOrAppendSiteGroupEntry(
// Update the storage data in |origin_size_map|. // Update the storage data in |origin_size_map|.
void UpdateDataForOrigin(const GURL& url, void UpdateDataForOrigin(const GURL& url,
const int size, const int64_t size,
std::map<std::string, int>* origin_size_map) { std::map<std::string, int64_t>* origin_size_map) {
if (size > 0) if (size > 0)
(*origin_size_map)[url.spec()] += size; (*origin_size_map)[url.spec()] += size;
} }
...@@ -296,7 +296,7 @@ bool IsPatternValidForType(const std::string& pattern_string, ...@@ -296,7 +296,7 @@ bool IsPatternValidForType(const std::string& pattern_string,
void UpdateDataFromCookiesTree( void UpdateDataFromCookiesTree(
std::map<std::string, std::set<std::string>>* all_sites_map, std::map<std::string, std::set<std::string>>* all_sites_map,
std::map<std::string, int>* origin_size_map, std::map<std::string, int64_t>* origin_size_map,
const GURL& origin, const GURL& origin,
int64_t size) { int64_t size) {
UpdateDataForOrigin(origin, size, origin_size_map); UpdateDataForOrigin(origin, size, origin_size_map);
...@@ -759,7 +759,7 @@ void SiteSettingsHandler::HandleGetAllSites(const base::ListValue* args) { ...@@ -759,7 +759,7 @@ void SiteSettingsHandler::HandleGetAllSites(const base::ListValue* args) {
} }
base::Value SiteSettingsHandler::PopulateCookiesAndUsageData(Profile* profile) { base::Value SiteSettingsHandler::PopulateCookiesAndUsageData(Profile* profile) {
std::map<std::string, int> origin_size_map; std::map<std::string, int64_t> origin_size_map;
std::map<std::string, int> origin_cookie_map; std::map<std::string, int> origin_cookie_map;
base::Value list_value(base::Value::Type::LIST); base::Value list_value(base::Value::Type::LIST);
...@@ -784,7 +784,7 @@ base::Value SiteSettingsHandler::PopulateCookiesAndUsageData(Profile* profile) { ...@@ -784,7 +784,7 @@ base::Value SiteSettingsHandler::PopulateCookiesAndUsageData(Profile* profile) {
const std::string& origin = origin_info.FindKey("origin")->GetString(); const std::string& origin = origin_info.FindKey("origin")->GetString();
const auto& size_info_it = origin_size_map.find(origin); const auto& size_info_it = origin_size_map.find(origin);
if (size_info_it != origin_size_map.end()) if (size_info_it != origin_size_map.end())
origin_info.SetKey("usage", base::Value(size_info_it->second)); origin_info.SetKey("usage", base::Value(double(size_info_it->second)));
const auto& origin_cookie_num_it = const auto& origin_cookie_num_it =
origin_cookie_map.find(GURL(origin).host()); origin_cookie_map.find(GURL(origin).host());
if (origin_cookie_num_it != origin_cookie_map.end()) { if (origin_cookie_num_it != origin_cookie_map.end()) {
...@@ -812,10 +812,10 @@ void SiteSettingsHandler::HandleGetFormattedBytes(const base::ListValue* args) { ...@@ -812,10 +812,10 @@ void SiteSettingsHandler::HandleGetFormattedBytes(const base::ListValue* args) {
CHECK_EQ(2U, args->GetSize()); CHECK_EQ(2U, args->GetSize());
const base::Value* callback_id; const base::Value* callback_id;
CHECK(args->Get(0, &callback_id)); CHECK(args->Get(0, &callback_id));
int num_bytes; double num_bytes;
CHECK(args->GetInteger(1, &num_bytes)); CHECK(args->GetDouble(1, &num_bytes));
const base::string16 string = ui::FormatBytes(num_bytes); const base::string16 string = ui::FormatBytes(int64_t(num_bytes));
ResolveJavascriptCallback(*callback_id, base::Value(string)); ResolveJavascriptCallback(*callback_id, base::Value(string));
} }
...@@ -1410,7 +1410,7 @@ void SiteSettingsHandler::TreeModelEndBatch(CookiesTreeModel* model) { ...@@ -1410,7 +1410,7 @@ void SiteSettingsHandler::TreeModelEndBatch(CookiesTreeModel* model) {
void SiteSettingsHandler::GetOriginStorage( void SiteSettingsHandler::GetOriginStorage(
std::map<std::string, std::set<std::string>>* all_sites_map, std::map<std::string, std::set<std::string>>* all_sites_map,
std::map<std::string, int>* origin_size_map) { std::map<std::string, int64_t>* origin_size_map) {
CHECK(cookies_tree_model_.get()); CHECK(cookies_tree_model_.get());
const CookieTreeNode* root = cookies_tree_model_->GetRoot(); const CookieTreeNode* root = cookies_tree_model_->GetRoot();
......
...@@ -121,6 +121,7 @@ class SiteSettingsHandler : public SettingsPageUIHandler, ...@@ -121,6 +121,7 @@ class SiteSettingsHandler : public SettingsPageUIHandler,
FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, ZoomLevels); FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, ZoomLevels);
FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest,
HandleClearEtldPlus1DataAndCookies); HandleClearEtldPlus1DataAndCookies);
FRIEND_TEST_ALL_PREFIXES(SiteSettingsHandlerTest, HandleGetFormattedBytes);
// Creates the CookiesTreeModel if necessary. // Creates the CookiesTreeModel if necessary.
void EnsureCookiesTreeModelCreated(); void EnsureCookiesTreeModelCreated();
...@@ -134,7 +135,7 @@ class SiteSettingsHandler : public SettingsPageUIHandler, ...@@ -134,7 +135,7 @@ class SiteSettingsHandler : public SettingsPageUIHandler,
// stores the information in the |all_sites_map| and |origin_size_map|. // stores the information in the |all_sites_map| and |origin_size_map|.
void GetOriginStorage( void GetOriginStorage(
std::map<std::string, std::set<std::string>>* all_sites_map, std::map<std::string, std::set<std::string>>* all_sites_map,
std::map<std::string, int>* origin_size_map); std::map<std::string, int64_t>* origin_size_map);
// Calculates the number of cookies for each etld+1 and each origin, and // Calculates the number of cookies for each etld+1 and each origin, and
// stores the information in the |all_sites_map| and |origin_cookie_map|. // stores the information in the |all_sites_map| and |origin_cookie_map|.
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/json/json_reader.h" #include "base/json/json_reader.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/simple_test_clock.h" #include "base/test/simple_test_clock.h"
#include "base/values.h" #include "base/values.h"
...@@ -50,6 +51,7 @@ ...@@ -50,6 +51,7 @@
#include "extensions/common/extension_builder.h" #include "extensions/common/extension_builder.h"
#include "ppapi/buildflags/buildflags.h" #include "ppapi/buildflags/buildflags.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/text/bytes_formatting.h"
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
#include "chrome/browser/chromeos/login/users/mock_user_manager.h" #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
...@@ -424,7 +426,7 @@ class SiteSettingsHandlerTest : public testing::Test { ...@@ -424,7 +426,7 @@ class SiteSettingsHandlerTest : public testing::Test {
url::Origin::Create(GURL("https://www.example.com/")), 2); url::Origin::Create(GURL("https://www.example.com/")), 2);
mock_browsing_data_local_storage_helper->AddLocalStorageForOrigin( mock_browsing_data_local_storage_helper->AddLocalStorageForOrigin(
url::Origin::Create(GURL("https://www.google.com/")), 5); url::Origin::Create(GURL("https://www.google.com/")), 50000000000);
mock_browsing_data_local_storage_helper->Notify(); mock_browsing_data_local_storage_helper->Notify();
mock_browsing_data_cookie_helper->AddCookieSamples( mock_browsing_data_cookie_helper->AddCookieSamples(
...@@ -761,7 +763,7 @@ TEST_F(SiteSettingsHandlerTest, OnStorageFetched) { ...@@ -761,7 +763,7 @@ TEST_F(SiteSettingsHandlerTest, OnStorageFetched) {
EXPECT_EQ("https://www.google.com/", EXPECT_EQ("https://www.google.com/",
origin_info->FindKey("origin")->GetString()); origin_info->FindKey("origin")->GetString());
EXPECT_EQ(0, origin_info->FindKey("engagement")->GetDouble()); EXPECT_EQ(0, origin_info->FindKey("engagement")->GetDouble());
EXPECT_EQ(5, origin_info->FindKey("usage")->GetDouble()); EXPECT_EQ(50000000000, origin_info->FindKey("usage")->GetDouble());
EXPECT_EQ(0, origin_info->FindKey("numCookies")->GetDouble()); EXPECT_EQ(0, origin_info->FindKey("numCookies")->GetDouble());
ASSERT_TRUE(storage_and_cookie_list->GetDictionary(2, &site_group)); ASSERT_TRUE(storage_and_cookie_list->GetDictionary(2, &site_group));
...@@ -1890,4 +1892,20 @@ TEST_F(SiteSettingsHandlerTest, HandleClearEtldPlus1DataAndCookies) { ...@@ -1890,4 +1892,20 @@ TEST_F(SiteSettingsHandlerTest, HandleClearEtldPlus1DataAndCookies) {
storage_and_cookie_list = GetOnStorageFetchedSentListValue(); storage_and_cookie_list = GetOnStorageFetchedSentListValue();
EXPECT_EQ(0U, storage_and_cookie_list->GetSize()); EXPECT_EQ(0U, storage_and_cookie_list->GetSize());
} }
TEST_F(SiteSettingsHandlerTest, HandleGetFormattedBytes) {
const double size = 120000000000;
base::ListValue get_args;
get_args.AppendString(kCallbackId);
get_args.AppendDouble(size);
handler()->HandleGetFormattedBytes(&get_args);
// Validate that this method can handle large data.
const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
EXPECT_EQ("cr.webUIResponse", data.function_name());
EXPECT_EQ(kCallbackId, data.arg1()->GetString());
ASSERT_TRUE(data.arg2()->GetBool());
EXPECT_EQ(base::UTF16ToUTF8(ui::FormatBytes(int64_t(size))),
data.arg3()->GetString());
}
} // namespace settings } // namespace settings
...@@ -241,6 +241,29 @@ suite('SiteEntry', function() { ...@@ -241,6 +241,29 @@ suite('SiteEntry', function() {
}); });
}); });
test(
'large number data usage shown correctly for grouped entries',
function() {
// Clone this object to avoid propagating changes made in this test.
const testSiteGroup =
JSON.parse(JSON.stringify(TEST_MULTIPLE_SITE_GROUP));
const numBytes1 = 2000000000;
const numBytes2 = 10000000000;
const numBytes3 = 7856;
testSiteGroup.origins[0].usage = numBytes1;
testSiteGroup.origins[1].usage = numBytes2;
testSiteGroup.origins[2].usage = numBytes3;
testElement.siteGroup = testSiteGroup;
Polymer.dom.flush();
return browserProxy.whenCalled('getFormattedBytes').then((args) => {
const sumBytes = numBytes1 + numBytes2 + numBytes3;
assertEquals(
`${sumBytes} B`,
testElement.root.querySelector('#displayName .data-unit')
.textContent.trim());
});
});
test('favicon with www.etld+1 chosen for site group', function() { test('favicon with www.etld+1 chosen for site group', function() {
// Clone this object to avoid propagating changes made in this test. // Clone this object to avoid propagating changes made in this test.
const testSiteGroup = JSON.parse(JSON.stringify(TEST_MULTIPLE_SITE_GROUP)); const testSiteGroup = JSON.parse(JSON.stringify(TEST_MULTIPLE_SITE_GROUP));
......
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