Skip to content

Commit

Permalink
[Provisioning] Change ResultCallback to take const ref results.
Browse files Browse the repository at this point in the history 
Bug: 1310686
Change-Id: I848ef52879c1c016817b0fdeafab64c9140c09f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3573965
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Melissa Zhang <melzhang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989300}
  • Loading branch information
@melzhan
melzhan authored and Chromium LUCI CQ committed Apr 6, 2022
1 parent 8a3627b commit 7dc0000
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 11 deletions.
4 changes: 2 additions & 2 deletions chrome/browser/apps/app_discovery_service/app_discovery_service_unittest.cc
View file
Expand Up @@ -74,7 +74,7 @@ TEST_F(AppDiscoveryServiceTest, GetAppsFromFetcherNoExtras) {

app_discovery_service->GetApps(
ResultType::kTestType,
base::BindLambdaForTesting([this](std::vector<Result> results,
base::BindLambdaForTesting([this](const std::vector<Result>& results,
DiscoveryError error) {
EXPECT_EQ(error, DiscoveryError::kSuccess);
EXPECT_EQ(results.size(), 1u);
Expand All @@ -101,7 +101,7 @@ TEST_F(AppDiscoveryServiceTest, GetArcAppsFromFetcher) {

app_discovery_service->GetApps(
ResultType::kTestType,
base::BindLambdaForTesting([this](std::vector<Result> results,
base::BindLambdaForTesting([this](const std::vector<Result>& results,
DiscoveryError error) {
EXPECT_EQ(error, DiscoveryError::kSuccess);
GURL kTestIconUrl(kTestPlayAppIconUrl);
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/apps/app_discovery_service/app_discovery_util.h
View file
Expand Up @@ -31,7 +31,8 @@ enum class DiscoveryError {
};

using ResultCallback =
base::OnceCallback<void(std::vector<Result> results, DiscoveryError error)>;
base::OnceCallback<void(const std::vector<Result>& results,
DiscoveryError error)>;

} // namespace apps

Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/apps/app_discovery_service/game_extras.cc
View file
Expand Up @@ -4,6 +4,8 @@

#include "chrome/browser/apps/app_discovery_service/game_extras.h"

#include <memory>

namespace apps {

GameExtras::GameExtras(
Expand All @@ -12,8 +14,14 @@ GameExtras::GameExtras(
const GURL& icon_url)
: platforms_(platforms), source_(source), icon_url_(icon_url) {}

GameExtras::GameExtras(const GameExtras&) = default;

GameExtras::~GameExtras() = default;

std::unique_ptr<SourceExtras> GameExtras::Clone() {
return std::make_unique<GameExtras>(*this);
}

const absl::optional<std::vector<std::u16string>>& GameExtras::GetPlatforms()
const {
return platforms_;
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/apps/app_discovery_service/game_extras.h
View file
Expand Up @@ -24,10 +24,12 @@ class GameExtras : public SourceExtras {
GameExtras(const absl::optional<std::vector<std::u16string>>& platforms,
Source source,
const GURL& icon_url);
GameExtras(const GameExtras&) = delete;
GameExtras(const GameExtras&);
GameExtras& operator=(const GameExtras&) = delete;
~GameExtras() override;

std::unique_ptr<SourceExtras> Clone() override;

const absl::optional<std::vector<std::u16string>>& GetPlatforms() const;
Source GetSource() const;
const GURL& GetIconUrl() const;
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/apps/app_discovery_service/play_extras.cc
View file
Expand Up @@ -4,6 +4,8 @@

#include "chrome/browser/apps/app_discovery_service/play_extras.h"

#include <memory>

namespace apps {

PlayExtras::PlayExtras(const std::string& package_name,
Expand All @@ -27,8 +29,14 @@ PlayExtras::PlayExtras(const std::string& package_name,
contains_ads_(contains_ads),
optimized_for_chrome_(optimized_for_chrome) {}

PlayExtras::PlayExtras(const PlayExtras&) = default;

PlayExtras::~PlayExtras() = default;

std::unique_ptr<SourceExtras> PlayExtras::Clone() {
return std::make_unique<PlayExtras>(*this);
}

const std::string& PlayExtras::GetPackageName() const {
return package_name_;
}
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/apps/app_discovery_service/play_extras.h
View file
Expand Up @@ -24,7 +24,7 @@ class PlayExtras : public SourceExtras {
const bool previously_installed,
const bool contains_ads,
const bool optimized_for_chrome);
PlayExtras(const PlayExtras&) = delete;
PlayExtras(const PlayExtras&);
PlayExtras& operator=(const PlayExtras&) = delete;
~PlayExtras() override;

Expand All @@ -42,6 +42,7 @@ class PlayExtras : public SourceExtras {
bool GetOptimizedForChrome() const;

// Result::SourceExtras:
std::unique_ptr<SourceExtras> Clone() override;
PlayExtras* AsPlayExtras() override;

private:
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/apps/app_discovery_service/recommended_arc_app_fetcher_unittest.cc
View file
Expand Up @@ -73,7 +73,7 @@ TEST_F(RecommendedArcAppFetcherTest, OnLoadSuccess) {
}
}]})json";
arc_app_fetcher()->SetCallbackForTesting(base::BindLambdaForTesting(
[](std::vector<Result> results, DiscoveryError error) {
[](const std::vector<Result>& results, DiscoveryError error) {
ASSERT_EQ(error, DiscoveryError::kSuccess);
ASSERT_EQ(results.size(), 1u);
EXPECT_EQ(results[0].GetAppSource(), AppSource::kPlay);
Expand Down Expand Up @@ -107,7 +107,7 @@ TEST_F(RecommendedArcAppFetcherTest, OnLoadSuccess) {

TEST_F(RecommendedArcAppFetcherTest, OnLoadError) {
arc_app_fetcher()->SetCallbackForTesting(base::BindLambdaForTesting(
[](std::vector<Result> results, DiscoveryError error) {
[](const std::vector<Result>& results, DiscoveryError error) {
ASSERT_EQ(results.size(), 0u);
ASSERT_EQ(error, DiscoveryError::kErrorRequestFailed);
}));
Expand All @@ -116,7 +116,7 @@ TEST_F(RecommendedArcAppFetcherTest, OnLoadError) {

TEST_F(RecommendedArcAppFetcherTest, OnParseResponseError) {
arc_app_fetcher()->SetCallbackForTesting(base::BindLambdaForTesting(
[](std::vector<Result> results, DiscoveryError error) {
[](const std::vector<Result>& results, DiscoveryError error) {
ASSERT_EQ(results.size(), 0u);
ASSERT_EQ(error, DiscoveryError::kErrorMalformedData);
}));
Expand Down
16 changes: 16 additions & 0 deletions chrome/browser/apps/app_discovery_service/result.cc
View file
Expand Up @@ -25,6 +25,22 @@ Result::Result(AppSource app_source,
app_title_(app_title),
source_extras_(std::move(source_extras)) {}

Result::Result(const Result& other)
: app_source_(other.app_source_),
app_id_(other.app_id_),
app_title_(other.app_title_),
source_extras_(other.source_extras_ ? other.source_extras_->Clone()
: nullptr) {}

Result& Result::operator=(const Result& other) {
app_source_ = other.app_source_;
app_id_ = other.app_id_;
app_title_ = other.app_title_;
source_extras_ =
other.source_extras_ ? other.source_extras_->Clone() : nullptr;
return *this;
}

Result::Result(Result&&) = default;

Result& Result::operator=(Result&&) = default;
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/apps/app_discovery_service/result.h
View file
Expand Up @@ -23,6 +23,8 @@ class SourceExtras {
// add a safe downcast here like so:
// virtual FooExtras* AsFooExtras { return nullptr; }

virtual std::unique_ptr<SourceExtras> Clone() = 0;

// Safe downcasts:
virtual GameExtras* AsGameExtras();
virtual PlayExtras* AsPlayExtras();
Expand All @@ -34,6 +36,8 @@ class Result {
const std::string& app_id,
const std::u16string& app_title,
std::unique_ptr<SourceExtras> source_extras);
Result(const Result&);
Result& operator=(const Result&);
Result(Result&&);
Result& operator=(Result&&);
~Result();
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ui/app_list/search/games/game_provider.cc
View file
Expand Up @@ -98,10 +98,11 @@ void GameProvider::UpdateIndex() {
weak_factory_.GetWeakPtr()));
}

void GameProvider::OnIndexUpdated(GameIndex index, apps::DiscoveryError error) {
void GameProvider::OnIndexUpdated(const GameIndex& index,
apps::DiscoveryError error) {
// TODO(crbug.com/1305880): Report the error to UMA.
if (!index.empty())
game_index_ = std::move(index);
game_index_ = index;
}

void GameProvider::Start(const std::u16string& query) {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/app_list/search/games/game_provider.h
View file
Expand Up @@ -47,7 +47,7 @@ class GameProvider : public SearchProvider {

private:
void UpdateIndex();
void OnIndexUpdated(GameIndex index, apps::DiscoveryError error);
void OnIndexUpdated(const GameIndex& index, apps::DiscoveryError error);
void OnSearchComplete(
std::u16string query,
std::vector<std::pair<const apps::Result*, double>> matches);
Expand Down

0 comments on commit 7dc0000

Please sign in to comment.