Skip to content

Commit

Permalink
delete shopping task module support
Browse files Browse the repository at this point in the history
The shopping module is no longer supported. With its
removal, we no longer need the idea of a "task module"
as it will only be a recipe module. Therefore, this CL
is meant to remove all task module type checks, since
we can always assume the type. It also changes all task
module tests to use the recipe module object shape.

A separate CL will be created to change the names of
functions and files to better fit with the above changes.
Everything for the "task_module" will be renamed to just
be for the "recipe_module".

Screenshots of recipes still showing after the changes:
Real data: screenshot/G44tZGnHSFYctTz.png
Fake data: screenshot/iXqQiBRSxn4WZ5q.png

Bug: 1304803
Change-Id: Ib4de8ec6f243e4ccdc5706fffa5731dac299d280
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3551583
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Sean Harrison <harrisonsean@chromium.org>
Reviewed-by: Mohamad Ahmadi <mahmadi@chromium.org>
Commit-Queue: Riley Tatum <rtatum@google.com>
Cr-Commit-Position: refs/heads/main@{#991276}
  • Loading branch information
Riley Tatum authored and Chromium LUCI CQ committed Apr 11, 2022
1 parent 09a58b6 commit 9cce852
Show file tree
Hide file tree
Showing 22 changed files with 233 additions and 561 deletions.
14 changes: 0 additions & 14 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1544,13 +1544,6 @@ const FeatureEntry::FeatureVariation kNtpRecipeTasksModuleVariations[] = {
std::size(kNtpRecipeTasksModuleFakeData), nullptr},
};

const FeatureEntry::FeatureParam kNtpShoppingTasksModuleFakeData[] = {
{ntp_features::kNtpShoppingTasksModuleDataParam, "fake"}};
const FeatureEntry::FeatureVariation kNtpShoppingTasksModuleVariations[] = {
{"- Fake Data", kNtpShoppingTasksModuleFakeData,
std::size(kNtpShoppingTasksModuleFakeData), nullptr},
};

const FeatureEntry::FeatureParam kNtpDriveModuleFakeData[] = {
{ntp_features::kNtpDriveModuleDataParam, "fake"}};
const FeatureEntry::FeatureParam kNtpDriveModuleManagedUsersOnly[] = {
Expand Down Expand Up @@ -5326,13 +5319,6 @@ const FeatureEntry kFeatureEntries[] = {
kNtpRecipeTasksModuleVariations,
"DesktopNtpModules")},

{"ntp-shopping-tasks-module",
flag_descriptions::kNtpShoppingTasksModuleName,
flag_descriptions::kNtpShoppingTasksModuleDescription, kOsDesktop,
FEATURE_WITH_PARAMS_VALUE_TYPE(ntp_features::kNtpShoppingTasksModule,
kNtpShoppingTasksModuleVariations,
"DesktopNtpModules")},

{"ntp-chrome-cart-module", flag_descriptions::kNtpChromeCartModuleName,
flag_descriptions::kNtpChromeCartModuleDescription, kOsDesktop,
FEATURE_WITH_PARAMS_VALUE_TYPE(ntp_features::kNtpChromeCartModule,
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/chrome_browser_interface_binders.cc
Original file line number Diff line number Diff line change
Expand Up @@ -856,8 +856,7 @@ void PopulateChromeWebUIFrameBinders(
NewTabPageUI>(map);
}

if (base::FeatureList::IsEnabled(ntp_features::kNtpRecipeTasksModule) ||
base::FeatureList::IsEnabled(ntp_features::kNtpShoppingTasksModule)) {
if (base::FeatureList::IsEnabled(ntp_features::kNtpRecipeTasksModule)) {
RegisterWebUIControllerInterfaceBinder<
task_module::mojom::TaskModuleHandler, NewTabPageUI>(map);
}
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -4297,11 +4297,6 @@
"owners": [ "bhatiarohit", "tiborg" ],
"expiry_milestone": 105
},
{
"name": "ntp-shopping-tasks-module",
"owners": [ "mahmadi", "tiborg" ],
"expiry_milestone": 100
},
{
"name": "ntp-view-hierarchy-repair",
"owners": [ "adamta", "sczs" ],
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3710,10 +3710,6 @@ const char kNtpRecipeTasksModuleName[] = "NTP Recipe Tasks Module";
const char kNtpRecipeTasksModuleDescription[] =
"Shows the recipe tasks module on the New Tab Page.";

const char kNtpShoppingTasksModuleName[] = "NTP Shopping Tasks Module";
const char kNtpShoppingTasksModuleDescription[] =
"Shows the shopping tasks module on the New Tab Page.";

const char kNtpChromeCartModuleName[] = "NTP Chrome Cart Module";
const char kNtpChromeCartModuleDescription[] =
"Shows the chrome cart module on the New Tab Page.";
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -2121,9 +2121,6 @@ extern const char kNtpPhotosModuleSoftOptOutDescription[];
extern const char kNtpRecipeTasksModuleName[];
extern const char kNtpRecipeTasksModuleDescription[];

extern const char kNtpShoppingTasksModuleName[];
extern const char kNtpShoppingTasksModuleDescription[];

extern const char kNtpChromeCartModuleName[];
extern const char kNtpChromeCartModuleDescription[];

Expand Down
16 changes: 5 additions & 11 deletions chrome/browser/new_tab_page/modules/task_module/task_module.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@ module task_module.mojom;

import "url/mojom/url.mojom";

// Available task modules.
enum TaskModuleType {
kRecipe,
kShopping,
};

// A task item such as recipe or product.
struct TaskItem {
// Human-readable name.
Expand Down Expand Up @@ -52,13 +46,13 @@ struct Task {
// Bound to the NTP WebUI handler.
interface TaskModuleHandler {
// Returns the primary task if available.
GetPrimaryTask(TaskModuleType task_module_type) => (Task? task);
GetPrimaryTask() => (Task? task);
// Dismisses the task with the given name and remembers that setting.
DismissTask(TaskModuleType task_module_type, string task_name);
DismissTask(string task_name);
// Restores the task with the given name and remembers that setting.
RestoreTask(TaskModuleType task_module_type, string task_name);
RestoreTask(string task_name);
// Logs that the task item at position |index| has been clicked.
OnTaskItemClicked(TaskModuleType task_module_type, uint32 index);
OnTaskItemClicked(uint32 index);
// Logs that the related search pill at position |index| has been clicked.
OnRelatedSearchClicked(TaskModuleType task_module_type, uint32 index);
OnRelatedSearchClicked(uint32 index);
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,8 @@
#include "chrome/browser/profiles/profile.h"

namespace {
const char* GetModuleName(task_module::mojom::TaskModuleType task_module_type) {
switch (task_module_type) {
case task_module::mojom::TaskModuleType::kRecipe:
return "RecipeTasks";
case task_module::mojom::TaskModuleType::kShopping:
return "ShoppingTasks";
default:
NOTREACHED();
}
const char* GetModuleName() {
return "RecipeTasks";
}
} // namespace

Expand All @@ -31,53 +24,29 @@ TaskModuleHandler::TaskModuleHandler(

TaskModuleHandler::~TaskModuleHandler() = default;

void TaskModuleHandler::GetPrimaryTask(
task_module::mojom::TaskModuleType task_module_type,
GetPrimaryTaskCallback callback) {
void TaskModuleHandler::GetPrimaryTask(GetPrimaryTaskCallback callback) {
TaskModuleServiceFactory::GetForProfile(profile_)->GetPrimaryTask(
task_module_type, std::move(callback));
std::move(callback));
}

void TaskModuleHandler::DismissTask(
task_module::mojom::TaskModuleType task_module_type,
const std::string& task_name) {
TaskModuleServiceFactory::GetForProfile(profile_)->DismissTask(
task_module_type, task_name);
void TaskModuleHandler::DismissTask(const std::string& task_name) {
TaskModuleServiceFactory::GetForProfile(profile_)->DismissTask(task_name);
}

void TaskModuleHandler::RestoreTask(
task_module::mojom::TaskModuleType task_module_type,
const std::string& task_name) {
TaskModuleServiceFactory::GetForProfile(profile_)->RestoreTask(
task_module_type, task_name);
void TaskModuleHandler::RestoreTask(const std::string& task_name) {
TaskModuleServiceFactory::GetForProfile(profile_)->RestoreTask(task_name);
}

void TaskModuleHandler::OnTaskItemClicked(
task_module::mojom::TaskModuleType task_module_type,
uint32_t index) {
std::string task_item_name;
switch (task_module_type) {
case task_module::mojom::TaskModuleType::kRecipe:
task_item_name = "Recipe";
break;
case task_module::mojom::TaskModuleType::kShopping:
task_item_name = "Product";
break;
default:
NOTREACHED();
}
void TaskModuleHandler::OnTaskItemClicked(uint32_t index) {
std::string task_item_name = "Recipe";
base::UmaHistogramCounts100(
base::StringPrintf("NewTabPage.%s.%sClick",
GetModuleName(task_module_type),
base::StringPrintf("NewTabPage.%s.%sClick", GetModuleName(),
task_item_name.c_str()),
index);
}

void TaskModuleHandler::OnRelatedSearchClicked(
task_module::mojom::TaskModuleType task_module_type,
uint32_t index) {
void TaskModuleHandler::OnRelatedSearchClicked(uint32_t index) {
base::UmaHistogramCounts100(
base::StringPrintf("NewTabPage.%s.RelatedSearchClick",
GetModuleName(task_module_type)),
base::StringPrintf("NewTabPage.%s.RelatedSearchClick", GetModuleName()),
index);
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,11 @@ class TaskModuleHandler : public task_module::mojom::TaskModuleHandler {
~TaskModuleHandler() override;

// task_module::mojom::TaskModuleHandler:
void GetPrimaryTask(task_module::mojom::TaskModuleType task_module_type,
GetPrimaryTaskCallback callback) override;
void DismissTask(task_module::mojom::TaskModuleType task_module_type,
const std::string& task_name) override;
void RestoreTask(task_module::mojom::TaskModuleType task_module_type,
const std::string& task_name) override;
void OnTaskItemClicked(task_module::mojom::TaskModuleType task_module_type,
uint32_t index) override;
void OnRelatedSearchClicked(
task_module::mojom::TaskModuleType task_module_type,
uint32_t index) override;
void GetPrimaryTask(GetPrimaryTaskCallback callback) override;
void DismissTask(const std::string& task_name) override;
void RestoreTask(const std::string& task_name) override;
void OnTaskItemClicked(uint32_t index) override;
void OnRelatedSearchClicked(uint32_t index) override;

private:
mojo::Receiver<task_module::mojom::TaskModuleHandler> receiver_;
Expand Down
Loading

0 comments on commit 9cce852

Please sign in to comment.