Commit 80dc12c0 authored by Min Qin's avatar Min Qin Committed by Commit Bot

Fix a crash when launching NTP after query tiles is enabled

Getting application locale may need to read disk, and that causes NTP to
crash

This CL fixes the issue by using the acceptLanguages instead, and
also fixes the TileManager logic to support AcceptLanguages

Bug: 1082649
Change-Id: I5212baf40d6f7cd0344d2dd0267eaafaf31156cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2200667Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#768772}
parent f438b18b
......@@ -97,8 +97,9 @@ std::unique_ptr<KeyedService> TileServiceFactory::BuildServiceInstanceFor(
auto* background_task_scheduler =
background_task::BackgroundTaskSchedulerFactory::GetForKey(key);
std::string accept_languanges = language::GetApplicationLocale(
ProfileKey::FromSimpleFactoryKey(key)->GetPrefs());
std::string accept_languanges =
ProfileKey::FromSimpleFactoryKey(key)->GetPrefs()->GetString(
language::prefs::kAcceptLanguages);
auto url_loader_factory =
SystemNetworkContextManager::GetInstance()->GetSharedURLLoaderFactory();
......
......@@ -8,6 +8,7 @@
#include "base/bind.h"
#include "base/memory/weak_ptr.h"
#include "base/strings/string_split.h"
#include "base/task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
......@@ -22,11 +23,11 @@ class TileManagerImpl : public TileManager {
public:
TileManagerImpl(std::unique_ptr<TileStore> store,
base::Clock* clock,
const std::string& locale)
const std::string& accept_languages)
: initialized_(false),
store_(std::move(store)),
clock_(clock),
locale_(locale) {}
accept_languages_(accept_languages) {}
TileManagerImpl(const TileManagerImpl& other) = delete;
TileManagerImpl& operator=(const TileManagerImpl& other) = delete;
......@@ -87,8 +88,9 @@ class TileManagerImpl : public TileManager {
FROM_HERE, base::BindOnce(std::move(callback), result_tile));
}
void SetLocaleForTesting(const std::string& locale) override {
locale_ = locale;
void SetAcceptLanguagesForTesting(
const std::string& accept_languages) override {
accept_languages_ = accept_languages;
}
void OnTileStoreInitialized(
......@@ -145,16 +147,21 @@ class TileManagerImpl : public TileManager {
// Check whether |locale_| matches with that of the |group|.
bool ValidateLocale(const TileGroup* group) const {
if (locale_.empty() || group->locale.empty())
if (accept_languages_.empty() || group->locale.empty())
return false;
// In case the language is en-GB or en-IN, consider
// In case the primary language matches (en-GB vs en-IN), consider
// those are matching.
std::string primary = locale_.substr(0, locale_.find("-"));
std::string group_primary =
group->locale.substr(0, group->locale.find("-"));
for (auto& lang :
base::SplitString(accept_languages_, ",", base::TRIM_WHITESPACE,
base::SPLIT_WANT_NONEMPTY)) {
if (lang.substr(0, lang.find("-")) == group_primary)
return true;
}
return primary == group_primary;
return false;
}
void OnGroupSaved(std::unique_ptr<TileGroup> group,
......@@ -194,8 +201,9 @@ class TileManagerImpl : public TileManager {
// Clock object.
base::Clock* clock_;
// Device locale, used to check if tiles stored are of the same language.
std::string locale_;
// Accept languages from the PrefService. Used to check if tiles stored are of
// the same language.
std::string accept_languages_;
base::WeakPtrFactory<TileManagerImpl> weak_ptr_factory_{this};
};
......
......@@ -31,7 +31,7 @@ class TileManager {
static std::unique_ptr<TileManager> Create(
std::unique_ptr<TileStore> tile_store,
base::Clock* clock,
const std::string& locale);
const std::string& accept_languages);
// Initializes the query tile store, loading them into memory after
// validating.
......@@ -47,7 +47,8 @@ class TileManager {
virtual void SaveTiles(std::unique_ptr<TileGroup> tile_group,
TileGroupStatusCallback callback) = 0;
virtual void SetLocaleForTesting(const std::string& locale) = 0;
virtual void SetAcceptLanguagesForTesting(
const std::string& accept_languages) = 0;
TileManager();
virtual ~TileManager() = default;
......
......@@ -356,16 +356,37 @@ TEST_F(TileManagerTest, GetTileById) {
GetSingleTile("id_not_exist", base::nullopt);
}
// Verify that GetTiles will return empty result if non-stored locale
// is passed.
TEST_F(TileManagerTest, GetTilesForNonStoredLocale) {
manager()->SetLocaleForTesting("zh");
// Verify that GetTiles will return empty result if no matching AcceptLanguages
// is found.
TEST_F(TileManagerTest, GetTilesWithoutMatchingAcceptLanguages) {
manager()->SetAcceptLanguagesForTesting("zh");
TileGroup group;
test::ResetTestGroup(&group);
InitWithData(TileGroupStatus::kSuccess, {group});
GetTiles(std::vector<Tile>());
}
// Verify that GetTiles will return a valid result if a matching AcceptLanguages
// is found.
TEST_F(TileManagerTest, GetTilesWithMatchingAcceptLanguages) {
manager()->SetAcceptLanguagesForTesting("zh, en");
TileGroup group;
group.id = "gid";
group.locale = "en-US";
group.last_updated_ts = clock()->Now();
auto tile = std::make_unique<Tile>();
test::ResetTestEntry(tile.get());
group.tiles.emplace_back(std::move(tile));
InitWithData(TileGroupStatus::kSuccess, {group});
auto expected_tile = std::make_unique<Tile>();
test::ResetTestEntry(expected_tile.get());
std::vector<Tile> expected;
expected.emplace_back(std::move(*expected_tile.get()));
GetTiles(std::move(expected));
}
} // namespace
} // namespace query_tiles
......@@ -35,7 +35,7 @@ class MockTileManager : public TileManager {
MOCK_METHOD2(GetTile, void(const std::string&, TileCallback));
MOCK_METHOD2(SaveTiles,
void(std::unique_ptr<TileGroup>, TileGroupStatusCallback));
MOCK_METHOD1(SetLocaleForTesting, void(const std::string&));
MOCK_METHOD1(SetAcceptLanguagesForTesting, void(const std::string&));
};
class MockBackgroundTaskScheduler
......
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