[ntp][modules] Filter out dismissed shopping tasks

Fixed: 1130861
Change-Id: I16f50fadf195a3ae6923e630781c36ba17ce7b1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2439558
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Auto-Submit: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813304}

chrome/browser/prefs/browser_prefs.cc

@@ -68,6 +68,7 @@
 #include "chrome/browser/push_messaging/push_messaging_app_identifier.h"
 #include "chrome/browser/rlz/chrome_rlz_tracker_delegate.h"
 #include "chrome/browser/search/search.h"
+#include "chrome/browser/search/shopping_tasks/shopping_tasks_service.h"
 #include "chrome/browser/sharing/sharing_sync_preference.h"
 #include "chrome/browser/ssl/ssl_config_service_manager.h"
 #include "chrome/browser/storage/appcache_feature_prefs.h"
@@ -895,6 +896,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry,
   settings::SettingsUI::RegisterProfilePrefs(registry);
   send_tab_to_self::SendTabToSelfBubbleController::RegisterProfilePrefs(
       registry);
+  ShoppingTasksService::RegisterProfilePrefs(registry);
   signin::RegisterProfilePrefs(registry);
   StartupBrowserCreator::RegisterProfilePrefs(registry);
   UnifiedAutoplayConfig::RegisterProfilePrefs(registry);

chrome/browser/resources/new_tab_page/modules/shopping_tasks/module.js

@@ -89,9 +89,15 @@ async function createModule() {
         element.showInfoDialog = true;
       },
       dismiss: () => {
+        ShoppingTasksHandlerProxy.getInstance().handler.dismissShoppingTask(
+            shoppingTask.name);
         return loadTimeData.getStringF(
             'dismissModuleToastMessage', shoppingTask.name);
       },
+      restore: () => {
+        ShoppingTasksHandlerProxy.getInstance().handler.restoreShoppingTask(
+            shoppingTask.name);
+      },
     },
   };
 }

chrome/browser/search/shopping_tasks/shopping_tasks.mojom

@@ -45,4 +45,8 @@ struct ShoppingTask {
 interface ShoppingTasksHandler {
   // Returns the primary shopping task if available.
   GetPrimaryShoppingTask() => (ShoppingTask? shopping_task);
+  // Dismisses the task with the given name and remembers that setting.
+  DismissShoppingTask(string task_name);
+  // Restores the task with the given name and remembers that setting.
+  RestoreShoppingTask(string task_name);
 };

chrome/browser/search/shopping_tasks/shopping_tasks_handler.cc

@@ -21,3 +21,13 @@ void ShoppingTasksHandler::GetPrimaryShoppingTask(
   ShoppingTasksServiceFactory::GetForProfile(profile_)->GetPrimaryShoppingTask(
       std::move(callback));
 }
+
+void ShoppingTasksHandler::DismissShoppingTask(const std::string& task_name) {
+  ShoppingTasksServiceFactory::GetForProfile(profile_)->DismissShoppingTask(
+      task_name);
+}
+
+void ShoppingTasksHandler::RestoreShoppingTask(const std::string& task_name) {
+  ShoppingTasksServiceFactory::GetForProfile(profile_)->RestoreShoppingTask(
+      task_name);
+}

chrome/browser/search/shopping_tasks/shopping_tasks_handler.h

@@ -25,6 +25,8 @@ class ShoppingTasksHandler
 
   // shopping_tasks::mojom::ShoppingTasksHandler:
   void GetPrimaryShoppingTask(GetPrimaryShoppingTaskCallback callback) override;
+  void DismissShoppingTask(const std::string& task_name) override;
+  void RestoreShoppingTask(const std::string& task_name) override;
 
  private:
   mojo::Receiver<shopping_tasks::mojom::ShoppingTasksHandler> receiver_;

chrome/browser/search/shopping_tasks/shopping_tasks_service.cc

@@ -4,8 +4,13 @@
 
 #include "chrome/browser/search/shopping_tasks/shopping_tasks_service.h"
 
+#include "base/stl_util.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/webui_url_constants.h"
 #include "components/google/core/common/google_util.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
 #include "net/base/url_util.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
@@ -14,6 +19,7 @@
 namespace {
 const char kNewTabShoppingTasksApiPath[] = "/async/newtab_shopping_tasks";
 const char kXSSIResponsePreamble[] = ")]}'";
+const char kDismissedTasksPrefName[] = "NewTabPage.DismissedShoppingTasks";
 
 GURL GetApiUrl(const std::string& application_locale) {
   GURL google_base_url = google_util::CommandLineGoogleBaseURL();
@@ -30,11 +36,17 @@ ShoppingTasksService::ShoppingTasksService(
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
     Profile* profile,
     const std::string& application_locale)
-    : url_loader_factory_(url_loader_factory),
+    : profile_(profile),
+      url_loader_factory_(url_loader_factory),
       application_locale_(application_locale) {}
 
 ShoppingTasksService::~ShoppingTasksService() = default;
 
+// static
+void ShoppingTasksService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
+  registry->RegisterListPref(kDismissedTasksPrefName);
+}
+
 void ShoppingTasksService::Shutdown() {}
 
 void ShoppingTasksService::GetPrimaryShoppingTask(
@@ -91,6 +103,16 @@ void ShoppingTasksService::GetPrimaryShoppingTask(
       network::SimpleURLLoader::kMaxBoundedStringDownloadSize);
 }
 
+void ShoppingTasksService::DismissShoppingTask(const std::string& task_name) {
+  ListPrefUpdate update(profile_->GetPrefs(), kDismissedTasksPrefName);
+  update->AppendIfNotPresent(std::make_unique<base::Value>(task_name));
+}
+
+void ShoppingTasksService::RestoreShoppingTask(const std::string& task_name) {
+  ListPrefUpdate update(profile_->GetPrefs(), kDismissedTasksPrefName);
+  update->EraseListValue(base::Value(task_name));
+}
+
 void ShoppingTasksService::OnDataLoaded(network::SimpleURLLoader* loader,
                                         ShoppingTaskCallback callback,
                                         std::unique_ptr<std::string> response) {
@@ -130,15 +152,17 @@ void ShoppingTasksService::OnJsonParsed(
     std::move(callback).Run(nullptr);
     return;
   }
-  auto* title = shopping_tasks->GetList()[0].FindStringPath("title");
-  auto* task_name = shopping_tasks->GetList()[0].FindStringPath("task_name");
-  auto* products = shopping_tasks->GetList()[0].FindListPath("products");
-  auto* related_searches =
-      shopping_tasks->GetList()[0].FindListPath("related_searches");
+  for (const auto& shopping_task : shopping_tasks->GetList()) {
+    auto* title = shopping_task.FindStringPath("title");
+    auto* task_name = shopping_task.FindStringPath("task_name");
+    auto* products = shopping_task.FindListPath("products");
+    auto* related_searches = shopping_task.FindListPath("related_searches");
     if (!title || !task_name || !products || !related_searches ||
         products->GetList().size() == 0) {
-    std::move(callback).Run(nullptr);
-    return;
+      continue;
+    }
+    if (IsShoppingTaskDismissed(*task_name)) {
+      continue;
     }
     std::vector<shopping_tasks::mojom::ProductPtr> mojo_products;
     for (const auto& product : products->GetList()) {
@@ -148,8 +172,7 @@ void ShoppingTasksService::OnJsonParsed(
       auto* info = product.FindStringPath("info");
       auto* target_url = product.FindStringPath("target_url");
       if (!name || !image_url || !price || !info || !target_url) {
-      std::move(callback).Run(nullptr);
-      return;
+        continue;
       }
       auto mojo_product = shopping_tasks::mojom::Product::New();
       mojo_product->name = *name;
@@ -164,8 +187,7 @@ void ShoppingTasksService::OnJsonParsed(
       auto* text = related_search.FindStringPath("text");
       auto* target_url = related_search.FindStringPath("target_url");
       if (!text || !target_url) {
-      std::move(callback).Run(nullptr);
-      return;
+        continue;
       }
       auto mojo_related_search = shopping_tasks::mojom::RelatedSearch::New();
       mojo_related_search->text = *text;
@@ -178,4 +200,16 @@ void ShoppingTasksService::OnJsonParsed(
     mojo_shopping_task->products = std::move(mojo_products);
     mojo_shopping_task->related_searches = std::move(mojo_related_searches);
     std::move(callback).Run(std::move(mojo_shopping_task));
+    return;
+  }
+  std::move(callback).Run(nullptr);
+}
+
+bool ShoppingTasksService::IsShoppingTaskDismissed(
+    const std::string& task_name) {
+  const base::ListValue* dismissed_tasks =
+      profile_->GetPrefs()->GetList(kDismissedTasksPrefName);
+  DCHECK(dismissed_tasks);
+  return dismissed_tasks->Find(base::Value(task_name)) !=
+         dismissed_tasks->end();
 }

chrome/browser/search/shopping_tasks/shopping_tasks_service.h

@@ -15,6 +15,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "services/data_decoder/public/cpp/data_decoder.h"
 
+class PrefRegistrySimple;
 class Profile;
 namespace network {
 class SharedURLLoaderFactory;
@@ -31,6 +32,8 @@ class ShoppingTasksService : public KeyedService {
   ShoppingTasksService(const ShoppingTasksService&) = delete;
   ~ShoppingTasksService() override;
 
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
+
   // KeyedService:
   void Shutdown() override;
 
@@ -38,9 +41,13 @@ class ShoppingTasksService : public KeyedService {
       shopping_tasks::mojom::ShoppingTaskPtr shopping_task)>;
   // Downloads and parses shopping tasks and calls |callback| when done.
   // On success |callback| is called with a populated |ShoppingTasksData| object
-  // of the highest priority shopping task. On failure, it is called with
-  // nullptr.
+  // of the first shopping task which has not been dismissed. On failure, it is
+  // called with nullptr.
   void GetPrimaryShoppingTask(ShoppingTaskCallback callback);
+  // Dismisses the task with the given name and remembers that setting.
+  void DismissShoppingTask(const std::string& task_name);
+  // Restores the task with the given name and remembers that setting.
+  void RestoreShoppingTask(const std::string& task_name);
 
  private:
   void OnDataLoaded(network::SimpleURLLoader* loader,
@@ -49,6 +56,10 @@ class ShoppingTasksService : public KeyedService {
   void OnJsonParsed(ShoppingTaskCallback callback,
                     data_decoder::DataDecoder::ValueOrError result);
 
+  // Returns whether a task with the given name has been dismissed.
+  bool IsShoppingTaskDismissed(const std::string& task_name);
+
+  Profile* profile_;
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
   std::list<std::unique_ptr<network::SimpleURLLoader>> loaders_;
   std::string application_locale_;

chrome/browser/search/shopping_tasks/shopping_tasks_service_unittest.cc

@@ -258,3 +258,127 @@ TEST_F(ShoppingTasksServiceTest, ErrorResponse) {
 
   ASSERT_FALSE(result);
 }
+
+// Verifies shopping tasks can be dismissed and restored and that the service
+// remembers not to return dismissed tasks.
+TEST_F(ShoppingTasksServiceTest, DismissTasks) {
+  test_url_loader_factory_.AddResponse(
+      "https://www.google.com/async/newtab_shopping_tasks?hl=en-US",
+      R"()]}'
+{
+  "update": {
+    "shopping_tasks": [
+      {
+        "title": "task 1 title",
+        "task_name": "task 1 name",
+        "products": [
+          {
+            "name": "foo",
+            "image_url": "https://foo.com",
+            "price": "$500",
+            "info": "visited 5 days ago",
+            "target_url": "https://google.com/foo"
+          }
+        ],
+        "related_searches": [
+          {
+            "text": "baz",
+            "target_url": "https://google.com/baz"
+          }
+        ]
+      },
+      {
+        "title": "task 2 title",
+        "task_name": "task 2 name",
+        "products": [
+          {
+            "name": "foo",
+            "image_url": "https://foo.com",
+            "price": "$500",
+            "info": "visited 5 days ago",
+            "target_url": "https://google.com/foo"
+          }
+        ],
+        "related_searches": [
+          {
+            "text": "baz",
+            "target_url": "https://google.com/baz"
+          }
+        ]
+      }
+    ]
+  }
+})");
+
+  shopping_tasks::mojom::ShoppingTaskPtr result1;
+  base::MockCallback<ShoppingTasksService::ShoppingTaskCallback> callback1;
+  EXPECT_CALL(callback1, Run(testing::_))
+      .Times(1)
+      .WillOnce(testing::Invoke(
+          [&result1](shopping_tasks::mojom::ShoppingTaskPtr arg) {
+            result1 = std::move(arg);
+          }));
+  service_->GetPrimaryShoppingTask(callback1.Get());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(result1);
+  EXPECT_EQ("task 1 name", result1->name);
+
+  service_->DismissShoppingTask("task 1 name");
+
+  shopping_tasks::mojom::ShoppingTaskPtr result2;
+  base::MockCallback<ShoppingTasksService::ShoppingTaskCallback> callback2;
+  EXPECT_CALL(callback2, Run(testing::_))
+      .Times(1)
+      .WillOnce(testing::Invoke(
+          [&result2](shopping_tasks::mojom::ShoppingTaskPtr arg) {
+            result2 = std::move(arg);
+          }));
+  service_->GetPrimaryShoppingTask(callback2.Get());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(result2);
+  EXPECT_EQ("task 2 name", result2->name);
+
+  service_->DismissShoppingTask("task 2 name");
+
+  shopping_tasks::mojom::ShoppingTaskPtr result3;
+  base::MockCallback<ShoppingTasksService::ShoppingTaskCallback> callback3;
+  EXPECT_CALL(callback3, Run(testing::_))
+      .Times(1)
+      .WillOnce(testing::Invoke(
+          [&result3](shopping_tasks::mojom::ShoppingTaskPtr arg) {
+            result3 = std::move(arg);
+          }));
+  service_->GetPrimaryShoppingTask(callback3.Get());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_FALSE(result3);
+
+  service_->RestoreShoppingTask("task 2 name");
+
+  shopping_tasks::mojom::ShoppingTaskPtr result4;
+  base::MockCallback<ShoppingTasksService::ShoppingTaskCallback> callback4;
+  EXPECT_CALL(callback4, Run(testing::_))
+      .Times(1)
+      .WillOnce(testing::Invoke(
+          [&result4](shopping_tasks::mojom::ShoppingTaskPtr arg) {
+            result4 = std::move(arg);
+          }));
+  service_->GetPrimaryShoppingTask(callback4.Get());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(result4);
+  EXPECT_EQ("task 2 name", result4->name);
+
+  service_->RestoreShoppingTask("task 1 name");
+
+  shopping_tasks::mojom::ShoppingTaskPtr result5;
+  base::MockCallback<ShoppingTasksService::ShoppingTaskCallback> callback5;
+  EXPECT_CALL(callback5, Run(testing::_))
+      .Times(1)
+      .WillOnce(testing::Invoke(
+          [&result5](shopping_tasks::mojom::ShoppingTaskPtr arg) {
+            result5 = std::move(arg);
+          }));
+  service_->GetPrimaryShoppingTask(callback5.Get());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(result5);
+  EXPECT_EQ("task 1 name", result5->name);
+}

chrome/test/data/webui/new_tab_page/modules/shopping_tasks/module_test.js

@@ -144,4 +144,65 @@ suite('NewTabPageModulesShoppingTasksModuleTest', () => {
     await checkHidden('700px', 26);
     await checkHidden('500px', 31);
   });
+
+  test('Backend is notified when module is dismissed or restored', async () => {
+    // Arrange.
+    const shoppingTask = {
+      title: 'Continue searching for Hello world',
+      name: 'Hello world',
+      products: [
+        {
+          name: 'foo',
+          imageUrl: {url: 'https://foo.com/img.png'},
+          price: '1 gazillion dollars',
+          info: 'foo info',
+          targetUrl: {url: 'https://foo.com'},
+        },
+        {
+          name: 'bar',
+          imageUrl: {url: 'https://bar.com/img.png'},
+          price: '2 gazillion dollars',
+          info: 'bar info',
+          targetUrl: {url: 'https://bar.com'},
+        },
+      ],
+      relatedSearches: [
+        {
+          text: 'baz',
+          targetUrl: {url: 'https://baz.com'},
+        },
+        {
+          text: 'blub',
+          targetUrl: {url: 'https://blub.com'},
+        },
+      ],
+    };
+    testProxy.handler.setResultFor(
+        'getPrimaryShoppingTask', Promise.resolve({shoppingTask}));
+
+
+    // Act.
+    await shoppingTasksDescriptor.initialize();
+
+    // Assert.
+    assertEquals('function', typeof shoppingTasksDescriptor.actions.dismiss);
+    assertEquals('function', typeof shoppingTasksDescriptor.actions.restore);
+
+    // Act.
+    const toastMessage = shoppingTasksDescriptor.actions.dismiss();
+
+    // Assert.
+    assertEquals('Removed Hello world', toastMessage);
+    assertEquals(
+        'Hello world',
+        await testProxy.handler.whenCalled('dismissShoppingTask'));
+
+    // Act.
+    shoppingTasksDescriptor.actions.restore();
+
+    // Assert.
+    assertEquals(
+        'Hello world',
+        await testProxy.handler.whenCalled('restoreShoppingTask'));
+  });
 });