Video Tutorials : Fixed summary card not getting shown

There was a bug due to which the tutorial manager didn't return the
summary card correctly. This CL fixes that.

Bug: 1166765
Change-Id: I0775f827f5ab96dd209b85c482ce33ef86ff70e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2629461
Auto-Submit: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843865}

chrome/browser/video_tutorials/internal/tutorial_manager.h

@@ -15,11 +15,16 @@ namespace video_tutorials {
 class TutorialManager {
  public:
   using SuccessCallback = base::OnceCallback<void(bool)>;
-  using GetTutorialsCallback = base::OnceCallback<void(std::vector<Tutorial>)>;
+  using MultipleItemCallback = base::OnceCallback<void(std::vector<Tutorial>)>;
+  using SingleItemCallback = base::OnceCallback<void(base::Optional<Tutorial>)>;
 
   // Loads video tutorials. Must be called again if the locale was changed by
   // the user.
-  virtual void GetTutorials(GetTutorialsCallback callback) = 0;
+  virtual void GetTutorials(MultipleItemCallback callback) = 0;
+
+  // Called to retrieve the tutorial associated with |feature_type|.
+  virtual void GetTutorial(FeatureType feature_type,
+                           SingleItemCallback callback) = 0;
 
   // Returns a list of languages for which video tutorials are available.
   virtual const std::vector<std::string>& GetSupportedLanguages() = 0;

chrome/browser/video_tutorials/internal/tutorial_manager_impl.cc

@@ -41,7 +41,7 @@ void TutorialManagerImpl::Initialize() {
                                     weak_ptr_factory_.GetWeakPtr()));
 }
 
-void TutorialManagerImpl::GetTutorials(GetTutorialsCallback callback) {
+void TutorialManagerImpl::GetTutorials(MultipleItemCallback callback) {
   if (!init_success_.has_value()) {
     MaybeCacheApiCall(base::BindOnce(&TutorialManagerImpl::GetTutorials,
                                      weak_ptr_factory_.GetWeakPtr(),
@@ -69,6 +69,33 @@ void TutorialManagerImpl::GetTutorials(GetTutorialsCallback callback) {
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
+void TutorialManagerImpl::GetTutorial(FeatureType feature_type,
+                                      SingleItemCallback callback) {
+  // Ensure that all the tutorials are already loaded.
+  GetTutorials(base::BindOnce(&TutorialManagerImpl::RunSingleItemCallback,
+                              weak_ptr_factory_.GetWeakPtr(),
+                              std::move(callback), feature_type));
+}
+
+void TutorialManagerImpl::RunSingleItemCallback(
+    SingleItemCallback callback,
+    FeatureType feature_type,
+    std::vector<Tutorial> tutorials_excluding_summary) {
+  if (!tutorial_group_.has_value()) {
+    std::move(callback).Run(base::nullopt);
+    return;
+  }
+
+  for (const Tutorial& tutorial : tutorial_group_->tutorials) {
+    if (tutorial.feature == feature_type) {
+      std::move(callback).Run(tutorial);
+      return;
+    }
+  }
+
+  std::move(callback).Run(base::nullopt);
+}
+
 const std::vector<std::string>& TutorialManagerImpl::GetSupportedLanguages() {
   return supported_languages_;
 }
@@ -113,7 +140,7 @@ void TutorialManagerImpl::OnInitialDataLoaded(
 }
 
 void TutorialManagerImpl::OnTutorialsLoaded(
-    GetTutorialsCallback callback,
+    MultipleItemCallback callback,
     bool success,
     std::unique_ptr<std::vector<TutorialGroup>> loaded_groups) {
   if (!success || !loaded_groups || loaded_groups->empty()) {

chrome/browser/video_tutorials/internal/tutorial_manager_impl.h

@@ -28,7 +28,9 @@ class TutorialManagerImpl : public TutorialManager {
 
  private:
   // TutorialManager implementation.
-  void GetTutorials(GetTutorialsCallback callback) override;
+  void GetTutorials(MultipleItemCallback callback) override;
+  void GetTutorial(FeatureType feature_type,
+                   SingleItemCallback callback) override;
   const std::vector<std::string>& GetSupportedLanguages() override;
   base::Optional<std::string> GetPreferredLocale() override;
   void SetPreferredLocale(const std::string& locale) override;
@@ -41,9 +43,12 @@ class TutorialManagerImpl : public TutorialManager {
       std::unique_ptr<std::vector<TutorialGroup>> all_groups);
   void MaybeCacheApiCall(base::OnceClosure api_call);
   void OnTutorialsLoaded(
-      GetTutorialsCallback callback,
+      MultipleItemCallback callback,
       bool success,
       std::unique_ptr<std::vector<TutorialGroup>> loaded_groups);
+  void RunSingleItemCallback(SingleItemCallback callback,
+                             FeatureType feature_type,
+                             std::vector<Tutorial> tutorials_excluding_summary);
 
   std::unique_ptr<TutorialStore> store_;
   PrefService* prefs_;

chrome/browser/video_tutorials/internal/tutorial_manager_impl_unittest.cc

@@ -132,6 +132,21 @@ class TutorialManagerTest : public testing::Test {
     std::move(closure).Run();
   }
 
+  void GetTutorial(FeatureType feature_type) {
+    base::RunLoop loop;
+    manager()->GetTutorial(
+        feature_type,
+        base::BindOnce(&TutorialManagerTest::OnGetTutorial,
+                       base::Unretained(this), loop.QuitClosure()));
+    loop.Run();
+  }
+
+  void OnGetTutorial(base::RepeatingClosure closure,
+                     base::Optional<Tutorial> tutorial) {
+    last_get_tutorial_result_ = tutorial;
+    std::move(closure).Run();
+  }
+
   void OnComplete(base::RepeatingClosure closure, bool success) {
     std::move(closure).Run();
   }
@@ -145,6 +160,9 @@ class TutorialManagerTest : public testing::Test {
   TutorialManager* manager() { return manager_.get(); }
   TestStore* tutorial_store() { return tutorial_store_; }
   std::vector<Tutorial> last_results() { return last_results_; }
+  base::Optional<Tutorial> last_get_tutorial_result() {
+    return last_get_tutorial_result_;
+  }
 
  private:
   base::test::TaskEnvironment task_environment_;
@@ -152,6 +170,7 @@ class TutorialManagerTest : public testing::Test {
   std::unique_ptr<TutorialManager> manager_;
   TestStore* tutorial_store_;
   std::vector<Tutorial> last_results_;
+  base::Optional<Tutorial> last_get_tutorial_result_;
 };
 
 TEST_F(TutorialManagerTest, InitAndGetTutorials) {
@@ -167,6 +186,9 @@ TEST_F(TutorialManagerTest, InitAndGetTutorials) {
   manager()->SetPreferredLocale("hi");
   GetTutorials();
   EXPECT_EQ(last_results().size(), 2u);
+
+  GetTutorial(FeatureType::kSearch);
+  EXPECT_EQ(FeatureType::kSearch, last_get_tutorial_result()->feature);
 }
 
 TEST_F(TutorialManagerTest, InitAndGetTutorialsWithSummary) {
@@ -184,6 +206,27 @@ TEST_F(TutorialManagerTest, InitAndGetTutorialsWithSummary) {
   EXPECT_EQ(last_results().size(), 2u);
 }
 
+TEST_F(TutorialManagerTest, GetSingleTutorialBeforeGetTutorialsCall) {
+  std::vector<FeatureType> features(
+      {FeatureType::kDownload, FeatureType::kSearch, FeatureType::kSummary});
+  auto groups = CreateSampleGroups({"hi", "kn"}, features);
+  auto tutorial_store = std::make_unique<StrictMock<TestStore>>();
+  tutorial_store->InitStoreData("hi", groups);
+  CreateTutorialManager(std::move(tutorial_store));
+
+  auto languages = manager()->GetSupportedLanguages();
+  EXPECT_EQ(languages.size(), 2u);
+  manager()->SetPreferredLocale("hi");
+  GetTutorial(FeatureType::kSummary);
+  EXPECT_TRUE(last_get_tutorial_result().has_value());
+  EXPECT_EQ(FeatureType::kSummary, last_get_tutorial_result()->feature);
+
+  GetTutorial(FeatureType::kSearch);
+  EXPECT_EQ(FeatureType::kSearch, last_get_tutorial_result()->feature);
+  GetTutorial(FeatureType::kVoiceSearch);
+  EXPECT_FALSE(last_get_tutorial_result().has_value());
+}
+
 TEST_F(TutorialManagerTest, SaveNewData) {
   std::vector<FeatureType> features(
       {FeatureType::kDownload, FeatureType::kSearch, FeatureType::kSummary});

chrome/browser/video_tutorials/internal/tutorial_service_impl.cc

@@ -34,22 +34,7 @@ void TutorialServiceImpl::GetTutorials(MultipleItemCallback callback) {
 
 void TutorialServiceImpl::GetTutorial(FeatureType feature_type,
                                       SingleItemCallback callback) {
-  tutorial_manager_->GetTutorials(base::BindOnce(
-      &TutorialServiceImpl::OnGetTutorials, weak_ptr_factory_.GetWeakPtr(),
-      std::move(callback), feature_type));
-}
-
-void TutorialServiceImpl::OnGetTutorials(SingleItemCallback callback,
-                                         FeatureType feature_type,
-                                         std::vector<Tutorial> tutorials) {
-  for (const Tutorial& tutorial : tutorials) {
-    if (tutorial.feature == feature_type) {
-      std::move(callback).Run(tutorial);
-      return;
-    }
-  }
-
-  std::move(callback).Run(base::nullopt);
+  tutorial_manager_->GetTutorial(feature_type, std::move(callback));
 }
 
 void TutorialServiceImpl::StartFetchIfNecessary() {

chrome/browser/video_tutorials/internal/tutorial_service_impl.h

@@ -30,10 +30,6 @@ class TutorialServiceImpl : public VideoTutorialService {
   void SetPreferredLocale(const std::string& locale) override;
 
  private:
-  void OnGetTutorials(SingleItemCallback callback,
-                      FeatureType feature_type,
-                      std::vector<Tutorial> tutorials);
-
   // Called at service startup to determine if a network fetch is necessary
   // based on the last fetch timestamp.
   void StartFetchIfNecessary();