GamesService: Parse GamesCatalog proto to get Highlighted Game.

Created a new DataFilesParser class which wraps the file IO operations,
allowing us to mock these calls in unit tests. The file reading
operation is kicked-off in a task posted on the thread pool to make
sure we're not blocking the calling thread.

In this CL, the highlighted game is simply the first game in the
catalog. A future CL will update that logic.

Bug: 1018201
Change-Id: Ic7bb5795344a7a3b4017587d7041ace77d921b00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928205
Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
Reviewed-by: Chris Sharp <csharp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719144}

components/games/core/BUILD.gn

@@ -4,6 +4,8 @@
 
 static_library("core") {
   sources = [
+    "data_files_parser.cc",
+    "data_files_parser.h",
     "games_constants.cc",
     "games_constants.h",
     "games_features.cc",
@@ -29,6 +31,7 @@ static_library("core") {
 source_set("unit_tests") {
   testonly = true
   sources = [
+    "data_files_parser_unittest.cc",
     "games_prefs_unittest.cc",
     "games_service_impl_unittest.cc",
   ]
@@ -37,6 +40,7 @@ source_set("unit_tests") {
     "//base",
     "//base/test:test_support",
     "//components/games/core/proto",
+    "//components/games/core/test:test_support",
     "//components/prefs:test_support",
     "//testing/gmock",
     "//testing/gtest",

components/games/core/data_files_parser.cc

+// Copyright 2019 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 "components/games/core/data_files_parser.h"
+
+#include "base/files/file_util.h"
+#include "components/games/core/games_utils.h"
+
+namespace games {
+
+namespace {
+
+const size_t kMaxCatalogSizeInBytes = 1024 * 1024;  // 1 MB
+
+}  //  namespace
+
+DataFilesParser::DataFilesParser() {}
+
+DataFilesParser::~DataFilesParser() = default;
+
+bool DataFilesParser::TryParseCatalog(const base::FilePath& install_dir,
+                                      GamesCatalog* out_catalog) {
+  const auto catalog_path = GetGamesCatalogPath(install_dir);
+
+  std::string file_content;
+  if (!base::ReadFileToStringWithMaxSize(catalog_path, &file_content,
+                                         kMaxCatalogSizeInBytes)) {
+    return false;
+  }
+
+  out_catalog->ParseFromString(file_content);
+  return true;
+}
+
+}  // namespace games

components/games/core/data_files_parser.h

+// Copyright 2019 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.
+
+#ifndef COMPONENTS_GAMES_CORE_DATA_FILES_PARSER_H_
+#define COMPONENTS_GAMES_CORE_DATA_FILES_PARSER_H_
+
+#include "base/files/file_path.h"
+#include "components/games/core/proto/games_catalog.pb.h"
+
+namespace games {
+
+class DataFilesParser {
+ public:
+  DataFilesParser();
+  virtual ~DataFilesParser();
+
+  virtual bool TryParseCatalog(const base::FilePath& install_dir,
+                               GamesCatalog* out_catalog);
+
+  DISALLOW_COPY_AND_ASSIGN(DataFilesParser);
+};
+
+}  // namespace games
+
+#endif  // COMPONENTS_GAMES_CORE_DATA_FILES_PARSER_H_

components/games/core/data_files_parser_unittest.cc

+// Copyright 2019 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 "components/games/core/data_files_parser.h"
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "components/games/core/games_utils.h"
+#include "components/games/core/proto/games_catalog.pb.h"
+#include "components/games/core/test/test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace games {
+
+class DataFilesParserTest : public testing::Test {
+ protected:
+  void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
+
+  DataFilesParser parser_;
+  base::ScopedTempDir temp_dir_;
+};
+
+TEST_F(DataFilesParserTest, TryParseCatalog_FileDoesNotExist) {
+  GamesCatalog catalog;
+  EXPECT_FALSE(parser_.TryParseCatalog(temp_dir_.GetPath(), &catalog));
+}
+
+TEST_F(DataFilesParserTest, TryParseCatalog_Success) {
+  GamesCatalog expected_catalog = test::CreateGamesCatalogWithOneGame();
+  std::string serialized_proto = expected_catalog.SerializeAsString();
+  const auto catalog_path = GetGamesCatalogPath(temp_dir_.GetPath());
+
+  ASSERT_NE(-1, base::WriteFile(catalog_path, serialized_proto.data(),
+                                serialized_proto.size()));
+
+  GamesCatalog test_catalog;
+  EXPECT_TRUE(parser_.TryParseCatalog(temp_dir_.GetPath(), &test_catalog));
+  EXPECT_TRUE(test::AreProtosEqual(expected_catalog, test_catalog));
+}
+
+}  // namespace games

components/games/core/games_service_impl.cc

@@ -6,29 +6,96 @@
 
 #include <memory>
 
+#include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/strings/string_piece.h"
+#include "base/task/post_task.h"
+#include "base/task_runner_util.h"
 #include "components/games/core/games_prefs.h"
 #include "components/games/core/proto/game.pb.h"
 
 namespace games {
 
-GamesServiceImpl::GamesServiceImpl(PrefService* prefs) : prefs_(prefs) {}
+GamesServiceImpl::GamesServiceImpl(PrefService* prefs)
+    : GamesServiceImpl(std::make_unique<DataFilesParser>(), prefs) {}
+
+GamesServiceImpl::GamesServiceImpl(
+    std::unique_ptr<DataFilesParser> files_parser,
+    PrefService* prefs)
+    : files_parser_(std::move(files_parser)),
+      prefs_(prefs),
+      task_runner_(
+          base::CreateSequencedTaskRunner({base::ThreadPool(), base::MayBlock(),
+                                           base::TaskPriority::USER_VISIBLE})) {
+}
 
 GamesServiceImpl::~GamesServiceImpl() = default;
 
 void GamesServiceImpl::GetHighlightedGame(HighlightedGameCallback callback) {
-  // If we don't have the install dir pref, then the Games component wasn't
-  // downloaded and we cannot provide the surface with a highlighted game.
-  base::FilePath data_file_path;
-  if (!prefs::TryGetInstallDirPath(prefs_, &data_file_path)) {
+  if (cached_highlighted_game_) {
+    std::move(callback).Run(ResponseCode::kSuccess,
+                            cached_highlighted_game_.get());
+    return;
+  }
+
+  // If the Games component wasn't downloaded, we cannot provide the surface
+  // with a highlighted game.
+  if (!IsComponentInstalled()) {
     std::move(callback).Run(ResponseCode::kFileNotFound, nullptr);
     return;
   }
 
-  auto game_proto = std::make_unique<Game>();
-  game_proto->set_title("Some Game!");
-  std::move(callback).Run(ResponseCode::kSuccess, std::move(game_proto));
+  base::PostTaskAndReplyWithResult(
+      task_runner_.get(), FROM_HERE,
+      base::BindOnce(&GamesServiceImpl::GetCatalog, base::Unretained(this)),
+      base::BindOnce(&GamesServiceImpl::GetHighlightedGameFromCatalog,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+std::unique_ptr<GamesCatalog> GamesServiceImpl::GetCatalog() {
+  // File IO should always be run using the thread pool task runner.
+  DCHECK(task_runner_->RunsTasksInCurrentSequence());
+
+  auto catalog = std::make_unique<GamesCatalog>();
+  if (!files_parser_->TryParseCatalog(*cached_data_files_dir_, catalog.get())) {
+    return nullptr;
+  }
+
+  return catalog;
+}
+
+void GamesServiceImpl::GetHighlightedGameFromCatalog(
+    HighlightedGameCallback callback,
+    std::unique_ptr<GamesCatalog> catalog) {
+  if (!catalog) {
+    std::move(callback).Run(ResponseCode::kFileNotFound, nullptr);
+    return;
+  }
+
+  if (catalog->games().empty()) {
+    std::move(callback).Run(ResponseCode::kInvalidData, nullptr);
+    return;
+  }
+
+  // Let's use the first game in the catalog as our highlighted game.
+  cached_highlighted_game_ = std::make_unique<const Game>(catalog->games(0));
+  std::move(callback).Run(ResponseCode::kSuccess,
+                          cached_highlighted_game_.get());
+}
+
+bool GamesServiceImpl::IsComponentInstalled() {
+  if (cached_highlighted_game_ && !cached_data_files_dir_->empty()) {
+    return true;
+  }
+
+  base::FilePath install_dir;
+  if (!prefs::TryGetInstallDirPath(prefs_, &install_dir)) {
+    return false;
+  }
+
+  cached_data_files_dir_ =
+      std::make_unique<base::FilePath>(std::move(install_dir));
+  return true;
 }
 
 }  // namespace games

components/games/core/games_service_impl.h

@@ -7,8 +7,13 @@
 
 #include <memory>
 
+#include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sequenced_task_runner.h"
+#include "components/games/core/data_files_parser.h"
 #include "components/games/core/games_service.h"
 #include "components/games/core/games_types.h"
+#include "components/games/core/proto/game.pb.h"
 #include "components/prefs/pref_service.h"
 
 namespace games {
@@ -16,12 +21,35 @@ namespace games {
 class GamesServiceImpl : public GamesService {
  public:
   explicit GamesServiceImpl(PrefService* prefs);
+
+  // Constructor to be used by unit tests.
+  explicit GamesServiceImpl(std::unique_ptr<DataFilesParser> files_parser,
+                            PrefService* prefs);
   ~GamesServiceImpl() override;
 
   void GetHighlightedGame(HighlightedGameCallback callback) override;
 
+ private:
+  void GetHighlightedGameFromCatalog(HighlightedGameCallback callback,
+                                     std::unique_ptr<GamesCatalog> catalog);
+
+  std::unique_ptr<GamesCatalog> GetCatalog();
+
+  bool IsComponentInstalled();
+
+  std::unique_ptr<DataFilesParser> files_parser_;
+
   // Will outlive the current instance.
   PrefService* prefs_;
+
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  std::unique_ptr<const Game> cached_highlighted_game_;
+  std::unique_ptr<const base::FilePath> cached_data_files_dir_;
+
+  base::WeakPtrFactory<GamesServiceImpl> weak_ptr_factory_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(GamesServiceImpl);
 };
 
 }  // namespace games

components/games/core/games_service_impl_unittest.cc

@@ -8,16 +8,35 @@
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
+#include "base/run_loop.h"
 #include "base/strings/string_piece.h"
 #include "base/test/bind_test_util.h"
+#include "base/test/task_environment.h"
+#include "components/games/core/data_files_parser.h"
 #include "components/games/core/games_prefs.h"
 #include "components/games/core/games_types.h"
 #include "components/games/core/proto/game.pb.h"
+#include "components/games/core/proto/games_catalog.pb.h"
+#include "components/games/core/test/test_utils.h"
 #include "components/prefs/testing_pref_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::testing::_;
+
 namespace games {
 
+namespace {
+
+class MockDataFilesParser : public DataFilesParser {
+ public:
+  ~MockDataFilesParser() override {}
+
+  MOCK_METHOD2(TryParseCatalog, bool(const base::FilePath&, GamesCatalog*));
+};
+
+}  // namespace
+
 class GamesServiceImplTest : public testing::Test {
  protected:
   void SetUp() override {
@@ -25,41 +44,137 @@ class GamesServiceImplTest : public testing::Test {
 
     games::prefs::RegisterProfilePrefs(test_pref_service_->registry());
 
-    games_service_ =
-        std::make_unique<GamesServiceImpl>(test_pref_service_.get());
+    auto mock_data_files_parser = std::make_unique<MockDataFilesParser>();
+    mock_data_files_parser_ = mock_data_files_parser.get();
+
+    games_service_ = std::make_unique<GamesServiceImpl>(
+        std::move(mock_data_files_parser), test_pref_service_.get());
+  }
+
+  void SetInstallDirPref() {
+    prefs::SetInstallDirPath(test_pref_service_.get(), fake_install_dir_);
   }
 
+  void AssertGetHighlightedGameFailsWith(ResponseCode expected_code) {
+    bool callback_called = false;
+
+    base::RunLoop run_loop;
+    games_service_->GetHighlightedGame(base::BindLambdaForTesting(
+        [&](ResponseCode given_code, const Game* given_game) {
+          EXPECT_EQ(expected_code, given_code);
+          callback_called = true;
+          run_loop.Quit();
+        }));
+
+    run_loop.Run();
+    EXPECT_TRUE(callback_called);
+  }
+
+  // TaskEnvironment is used instead of SingleThreadTaskEnvironment since we
+  // post a task to the thread pool.
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+
+  MockDataFilesParser* mock_data_files_parser_;
   std::unique_ptr<TestingPrefServiceSimple> test_pref_service_;
   std::unique_ptr<GamesServiceImpl> games_service_;
+  base::FilePath fake_install_dir_ =
+      base::FilePath(FILE_PATH_LITERAL("some/path"));
 };
 
-TEST_F(GamesServiceImplTest, GetHighlightedGame_SameTitle) {
-  prefs::SetInstallDirPath(test_pref_service_.get(),
-                           base::FilePath(FILE_PATH_LITERAL("some/path")));
+TEST_F(GamesServiceImplTest, GetHighlightedGame_ReturnsFirstFromCatalog) {
+  SetInstallDirPref();
+
+  // Create two games with different IDs, to make sure we can validate which one
+  // was picked.
+  GamesCatalog fake_catalog = test::CreateGamesCatalog(
+      {test::CreateGame(/*id=*/1), test::CreateGame(/*id=*/2)});
+  const Game& fake_highlighted_game = fake_catalog.games(0);
 
-  std::string expected_title = "Some Game!";
-  std::string result_title = "not the same";
+  EXPECT_CALL(*mock_data_files_parser_, TryParseCatalog(fake_install_dir_, _))
+      .WillOnce([&fake_catalog](const base::FilePath& install_dir,
+                                GamesCatalog* out_catalog) {
+        *out_catalog = fake_catalog;
+        return true;
+      });
 
+  const Game* result_game;
+
+  base::RunLoop run_loop;
   games_service_->GetHighlightedGame(base::BindLambdaForTesting(
-      [&](ResponseCode code, std::unique_ptr<Game> given_game) {
+      [&](ResponseCode code, const Game* given_game) {
         ASSERT_EQ(ResponseCode::kSuccess, code);
-        ASSERT_NE(nullptr, given_game);
-        result_title = given_game->title();
+        result_game = given_game;
+        run_loop.Quit();
       }));
 
-  ASSERT_EQ(expected_title, result_title);
+  run_loop.Run();
+
+  EXPECT_NE(nullptr, result_game);
+  EXPECT_TRUE(test::AreProtosEqual(fake_highlighted_game, *result_game));
 }
 
-TEST_F(GamesServiceImplTest, MissingFilePaths_FileNotFoundError) {
-  bool callback_called = false;
+TEST_F(GamesServiceImplTest, GetHighlightedGame_CachesHighlightedGame) {
+  SetInstallDirPref();
 
-  games_service_->GetHighlightedGame(base::BindLambdaForTesting(
-      [&](ResponseCode code, std::unique_ptr<Game> given_game) {
-        ASSERT_EQ(ResponseCode::kFileNotFound, code);
-        callback_called = true;
-      }));
+  GamesCatalog fake_catalog = test::CreateGamesCatalogWithOneGame();
+  const Game& fake_highlighted_game = fake_catalog.games(0);
+
+  // Since caching is supposed to be working, we only expect the catalog to be
+  // retrieved once.
+  EXPECT_CALL(*mock_data_files_parser_, TryParseCatalog(fake_install_dir_, _))
+      .Times(1)
+      .WillOnce([&fake_catalog](const base::FilePath& install_dir,
+                                GamesCatalog* out_catalog) {
+        *out_catalog = fake_catalog;
+        return true;
+      });
+
+  // Call GetHighlightedGame twice.
+  for (int i = 0; i < 2; i++) {
+    const Game* result_game;
+    base::RunLoop run_loop;
+    games_service_->GetHighlightedGame(base::BindLambdaForTesting(
+        [&](ResponseCode code, const Game* given_game) {
+          EXPECT_EQ(ResponseCode::kSuccess, code);
+          result_game = given_game;
+          run_loop.Quit();
+        }));
+
+    run_loop.Run();
+
+    EXPECT_NE(nullptr, result_game);
+    EXPECT_TRUE(test::AreProtosEqual(fake_highlighted_game, *result_game));
+  }
+}
+
+TEST_F(GamesServiceImplTest, GetHighlightedGame_ComponentNotInstalled) {
+  AssertGetHighlightedGameFailsWith(ResponseCode::kFileNotFound);
+}
+
+TEST_F(GamesServiceImplTest, GetHighlightedGame_CatalogNotFound) {
+  SetInstallDirPref();
+
+  EXPECT_CALL(*mock_data_files_parser_, TryParseCatalog(fake_install_dir_, _))
+      .WillOnce([](const base::FilePath& install_dir,
+                   GamesCatalog* out_catalog) { return false; });
+
+  AssertGetHighlightedGameFailsWith(ResponseCode::kFileNotFound);
+}
+
+TEST_F(GamesServiceImplTest, GetHighlightedGame_CatalogEmpty) {
+  SetInstallDirPref();
+
+  GamesCatalog empty_catalog;
+
+  EXPECT_CALL(*mock_data_files_parser_, TryParseCatalog(fake_install_dir_, _))
+      .WillOnce([&empty_catalog](const base::FilePath& install_dir,
+                                 GamesCatalog* out_catalog) {
+        *out_catalog = empty_catalog;
+        return true;
+      });
 
-  ASSERT_TRUE(callback_called);
+  AssertGetHighlightedGameFailsWith(ResponseCode::kInvalidData);
 }
 
 }  // namespace games

components/games/core/games_types.h

@@ -10,16 +10,20 @@
 
 #include "base/callback.h"
 #include "components/games/core/proto/game.pb.h"
+#include "components/games/core/proto/games_catalog.pb.h"
 
 namespace games {
 
 enum ResponseCode {
   kSuccess = 0,
   kFileNotFound = 1,
+  kInvalidData = 2,
 };
 
+using GamesCatalogCallback =
+    base::OnceCallback<void(ResponseCode, std::unique_ptr<GamesCatalog>)>;
 using HighlightedGameCallback =
-    base::OnceCallback<void(ResponseCode, std::unique_ptr<Game>)>;
+    base::OnceCallback<void(ResponseCode, const Game*)>;
 
 }  // namespace games
 

components/games/core/test/BUILD.gn

+# Copyright 2019 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.
+
+static_library("test_support") {
+  testonly = true
+  sources = [
+    "test_utils.cc",
+    "test_utils.h",
+  ]
+  deps = [
+    "//components/games/core/proto",
+  ]
+}

components/games/core/test/test_utils.cc

+// Copyright 2019 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 "components/games/core/test/test_utils.h"
+
+namespace games {
+namespace test {
+
+GamesCatalog CreateGamesCatalog(std::vector<Game> games) {
+  GamesCatalog catalog;
+
+  for (auto& game : games) {
+    catalog.mutable_games()->Add(std::move(game));
+  }
+
+  return catalog;
+}
+
+GamesCatalog CreateGamesCatalogWithOneGame() {
+  return CreateGamesCatalog({CreateGame()});
+}
+
+Game CreateGame(int id) {
+  Game game;
+  game.set_id(id);
+  game.set_title("Snake");
+  game.set_url("https://www.google.com/some/game");
+
+  GameImage game_image;
+  game_image.set_type(GameImageType::GAME_IMAGE_TYPE_CARD);
+  game_image.set_url("https://www.google.com/some/card/image");
+
+  game.mutable_images()->Add(std::move(game_image));
+  return game;
+}
+
+bool AreProtosEqual(const google::protobuf::MessageLite& lhs,
+                    const google::protobuf::MessageLite& rhs) {
+  return lhs.SerializeAsString() == rhs.SerializeAsString();
+}
+
+}  // namespace test
+}  // namespace games

components/games/core/test/test_utils.h

+// Copyright 2019 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.
+
+#ifndef COMPONENTS_GAMES_CORE_TEST_TEST_UTILS_H_
+#define COMPONENTS_GAMES_CORE_TEST_TEST_UTILS_H_
+
+#include <vector>
+
+#include "components/games/core/proto/game.pb.h"
+#include "components/games/core/proto/game_image.pb.h"
+#include "components/games/core/proto/games_catalog.pb.h"
+
+namespace games {
+namespace test {
+
+GamesCatalog CreateGamesCatalog(std::vector<Game> games);
+
+GamesCatalog CreateGamesCatalogWithOneGame();
+
+Game CreateGame(int id = 1);
+
+bool AreProtosEqual(const google::protobuf::MessageLite& lhs,
+                    const google::protobuf::MessageLite& rhs);
+
+}  // namespace test
+}  // namespace games
+
+#endif  // COMPONENTS_GAMES_CORE_TEST_TEST_UTILS_H_