[IntentHandling] Delete an App from preferred apps.

Implement delete app from preferred apps for an |app_id|
and |intent_filter|.

BUG=853604

Change-Id: Ibc1060304765813bcde50f80d1a00a0d10fee880
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1910823Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714812}

chrome/services/app_service/public/cpp/preferred_apps.cc

@@ -244,6 +244,64 @@ void SetPreferredApp(const std::vector<apps::mojom::ConditionPtr>& conditions,
   }
 }
 
+// Similar to SetPreferredApp(), this method go through every combination of
+// the condition values to clear app_id.
+void RemovePreferredApp(
+    const std::vector<apps::mojom::ConditionPtr>& conditions,
+    size_t index,
+    base::Value* dict,
+    const std::string& app_id) {
+  // If there are no more condition key to add to the dictionary, we reach the
+  // base case, delete the key_value pair if the stored app id is the same as
+  // |app_id|.
+  if (index == conditions.size()) {
+    const std::string* app_id_found = dict->FindStringKey(kAppId);
+    if (app_id_found && *app_id_found == app_id) {
+      dict->RemoveKey(kAppId);
+    }
+    return;
+  }
+
+  const auto& condition = conditions[index];
+  auto* condition_type_dict =
+      dict->FindKey(ConditionTypeToString(condition->condition_type));
+  if (!condition_type_dict) {
+    return;
+  }
+  for (const auto& condition_value : condition->condition_values) {
+    std::string condition_value_key = condition_value->value;
+    base::Value* condition_value_dictionary;
+    // For pattern type, use two nested dictionaries to represent the pattern
+    // and the match type.
+    if (condition->condition_type == apps::mojom::ConditionType::kPattern) {
+      auto* match_type_dict = condition_type_dict->FindKey(condition_value_key);
+      if (!match_type_dict) {
+        continue;
+      }
+      condition_value_dictionary = match_type_dict->FindKey(
+          MatchTypeToString(condition_value->match_type));
+    } else {
+      condition_value_dictionary =
+          condition_type_dict->FindKey(condition_value_key);
+    }
+    if (!condition_value_dictionary) {
+      continue;
+    }
+    // For each |condition_value|, search dictionary for the following
+    // conditions.
+    RemovePreferredApp(conditions, index + 1, condition_value_dictionary,
+                       app_id);
+    // Clean up empty dictionary after remove the app id.
+    if (condition_value_dictionary->DictEmpty()) {
+      condition_type_dict->RemoveKey(condition_value_key);
+    }
+  }
+  // Clean up the empty dictionary if there is no content left.
+  if (condition_type_dict->DictEmpty()) {
+    dict->RemoveKey(ConditionTypeToString(condition->condition_type));
+  }
+}
+
 }  // namespace
 
 namespace apps {
@@ -295,6 +353,24 @@ bool PreferredApps::AddPreferredApp(
   return true;
 }
 
+// static
+// Delete a preferred app for a preferred app dictionary.
+bool PreferredApps::DeletePreferredApp(
+    const std::string& app_id,
+    const apps::mojom::IntentFilterPtr& intent_filter,
+    base::Value* preferred_apps) {
+  if (!preferred_apps) {
+    return false;
+  }
+
+  // For an |intent_filter| there could be multiple |conditions|, and for each
+  // condition, there could be multiple |condition_values|. When we remove
+  // preferred app for and |intent_filter|, we need to remove the preferred app
+  // for all combinations of these |condition_values|.
+  RemovePreferredApp(intent_filter->conditions, 0, preferred_apps, app_id);
+  return true;
+}
+
 void PreferredApps::Init(std::unique_ptr<base::Value> preferred_apps) {
   if (preferred_apps && VerifyPreferredApps(preferred_apps.get())) {
     preferred_apps_ = std::move(preferred_apps);
@@ -313,6 +389,15 @@ bool PreferredApps::AddPreferredApp(
   return AddPreferredApp(app_id, intent_filter, preferred_apps_.get());
 }
 
+bool PreferredApps::DeletePreferredApp(
+    const std::string& app_id,
+    const apps::mojom::IntentFilterPtr& intent_filter) {
+  if (!preferred_apps_) {
+    return false;
+  }
+  return DeletePreferredApp(app_id, intent_filter, preferred_apps_.get());
+}
+
 base::Optional<std::string> PreferredApps::FindPreferredAppForIntent(
     const apps::mojom::IntentPtr& intent) {
   base::Optional<std::string> best_match_app_id = base::nullopt;

chrome/services/app_service/public/cpp/preferred_apps.h

@@ -50,12 +50,22 @@ class PreferredApps {
                               const apps::mojom::IntentFilterPtr& intent_filter,
                               base::Value* preferred_apps);
 
+  // Delete a preferred app for an |intent_filter| for |preferred_apps|.
+  static bool DeletePreferredApp(
+      const std::string& app_id,
+      const apps::mojom::IntentFilterPtr& intent_filter,
+      base::Value* preferred_apps);
+
   void Init(std::unique_ptr<base::Value> preferred_apps);
 
   // Add a preferred app for an |intent_filter|.
   bool AddPreferredApp(const std::string& app_id,
                        const apps::mojom::IntentFilterPtr& intent_filter);
 
+  // Delete a preferred app for an |intent_filter|.
+  bool DeletePreferredApp(const std::string& app_id,
+                          const apps::mojom::IntentFilterPtr& intent_filter);
+
   // Find preferred app id for an |intent|.
   base::Optional<std::string> FindPreferredAppForIntent(
       const apps::mojom::IntentPtr& intent);

chrome/services/app_service/public/cpp/preferred_apps_unittest.cc

@@ -32,6 +32,7 @@ class PreferredAppTest : public testing::Test {
 
     return intent_filter;
   }
+
   apps::mojom::IntentFilterPtr CreateSchemeAndHostOnlyFilter(
       const std::string& scheme,
       const std::string& host) {
@@ -55,6 +56,21 @@ class PreferredAppTest : public testing::Test {
     return intent_filter;
   }
 
+  apps::mojom::IntentFilterPtr CreatePatternFilter(
+      const std::string& pattern,
+      apps::mojom::PatternMatchType match_type) {
+    auto intent_filter =
+        CreateSchemeAndHostOnlyFilter("https", "www.google.com");
+    auto pattern_condition =
+        apps_util::MakeCondition(apps::mojom::ConditionType::kPattern,
+                                 std::vector<apps::mojom::ConditionValuePtr>());
+    intent_filter->conditions.push_back(std::move(pattern_condition));
+    auto condition_value = apps_util::MakeConditionValue(pattern, match_type);
+    intent_filter->conditions[2]->condition_values.push_back(
+        std::move(condition_value));
+    return intent_filter;
+  }
+
   apps::PreferredApps preferred_apps_;
 };
 
@@ -188,33 +204,16 @@ TEST_F(PreferredAppTest, DifferentPatterns) {
   preferred_apps_.Init(
       std::make_unique<base::Value>(base::Value::Type::DICTIONARY));
 
-  auto intent_filter = CreateSchemeAndHostOnlyFilter("https", "www.google.com");
+  auto intent_filter_literal =
+      CreatePatternFilter("/bc", apps::mojom::PatternMatchType::kLiteral);
+  auto intent_filter_prefix =
+      CreatePatternFilter("/a", apps::mojom::PatternMatchType::kPrefix);
+  auto intent_filter_glob =
+      CreatePatternFilter("/c.*d", apps::mojom::PatternMatchType::kGlob);
 
-  auto pattern_condition =
-      apps_util::MakeCondition(apps::mojom::ConditionType::kPattern,
-                               std::vector<apps::mojom::ConditionValuePtr>());
-  intent_filter->conditions.push_back(std::move(pattern_condition));
-
-  auto condition_value_literal = apps_util::MakeConditionValue(
-      "/bc", apps::mojom::PatternMatchType::kLiteral);
-  auto condition_value_prefix = apps_util::MakeConditionValue(
-      "/a", apps::mojom::PatternMatchType::kPrefix);
-  auto condition_value_glob = apps_util::MakeConditionValue(
-      "/c.*d", apps::mojom::PatternMatchType::kGlob);
-
-  intent_filter->conditions[2]->condition_values.push_back(
-      std::move(condition_value_literal));
-  preferred_apps_.AddPreferredApp(kAppId1, intent_filter);
-
-  intent_filter->conditions[2]->condition_values.clear();
-  intent_filter->conditions[2]->condition_values.push_back(
-      std::move(condition_value_prefix));
-  preferred_apps_.AddPreferredApp(kAppId2, intent_filter);
-
-  intent_filter->conditions[2]->condition_values.clear();
-  intent_filter->conditions[2]->condition_values.push_back(
-      std::move(condition_value_glob));
-  preferred_apps_.AddPreferredApp(kAppId3, intent_filter);
+  preferred_apps_.AddPreferredApp(kAppId1, intent_filter_literal);
+  preferred_apps_.AddPreferredApp(kAppId2, intent_filter_prefix);
+  preferred_apps_.AddPreferredApp(kAppId3, intent_filter_glob);
 
   GURL url_1 = GURL("https://www.google.com/bc");
   GURL url_2 = GURL("https://www.google.com/abbb");
@@ -282,3 +281,124 @@ TEST_F(PreferredAppTest, VerifyPreferredApps) {
   efg_dict->SetKey("klm", base::ListValue());
   EXPECT_FALSE(apps::PreferredApps::VerifyPreferredApps(&preferred_app));
 }
+
+// Test that for a single preferred app with URL filter, we can delete
+// the preferred app id.
+TEST_F(PreferredAppTest, DeletePreferredAppForURL) {
+  preferred_apps_.Init(
+      std::make_unique<base::Value>(base::Value::Type::DICTIONARY));
+  GURL filter_url = GURL("https://www.google.com/abc");
+  auto intent_filter = apps_util::CreateIntentFilterForUrlScope(filter_url);
+  preferred_apps_.AddPreferredApp(kAppId1, intent_filter);
+
+  EXPECT_EQ(kAppId1, preferred_apps_.FindPreferredAppForUrl(filter_url));
+
+  // If try to delete with wrong ID, won't delete.
+  preferred_apps_.DeletePreferredApp(kAppId2, intent_filter);
+  EXPECT_EQ(kAppId1, preferred_apps_.FindPreferredAppForUrl(filter_url));
+
+  preferred_apps_.DeletePreferredApp(kAppId1, intent_filter);
+  EXPECT_EQ(base::nullopt, preferred_apps_.FindPreferredAppForUrl(filter_url));
+  EXPECT_TRUE(preferred_apps_.GetValue().DictEmpty());
+}
+
+// Test for preferred app with filter that does not have all condition
+// types. E.g. delete preferred app with intent filter that only have scheme.
+TEST_F(PreferredAppTest, DeleteForTopLayerFilters) {
+  preferred_apps_.Init(
+      std::make_unique<base::Value>(base::Value::Type::DICTIONARY));
+  auto intent_filter = CreateSchemeOnlyFilter("tel");
+  preferred_apps_.AddPreferredApp(kAppId1, intent_filter);
+
+  GURL url_in_scope = GURL("tel://1234556/");
+  EXPECT_EQ(kAppId1, preferred_apps_.FindPreferredAppForUrl(url_in_scope));
+
+  preferred_apps_.DeletePreferredApp(kAppId1, intent_filter);
+  EXPECT_EQ(base::nullopt,
+            preferred_apps_.FindPreferredAppForUrl(url_in_scope));
+}
+
+// Test that we can properly delete for filters that has multiple
+// condition values for a condition type.
+TEST_F(PreferredAppTest, DeleteMultipleConditionValues) {
+  preferred_apps_.Init(
+      std::make_unique<base::Value>(base::Value::Type::DICTIONARY));
+
+  auto intent_filter =
+      apps_util::CreateIntentFilterForUrlScope(GURL("https://www.google.com/"));
+  intent_filter->conditions[0]->condition_values.push_back(
+      apps_util::MakeConditionValue("http",
+                                    apps::mojom::PatternMatchType::kNone));
+
+  preferred_apps_.AddPreferredApp(kAppId1, intent_filter);
+
+  GURL url_https = GURL("https://www.google.com/");
+  GURL url_http = GURL("http://www.google.com/");
+  EXPECT_EQ(kAppId1, preferred_apps_.FindPreferredAppForUrl(url_https));
+  EXPECT_EQ(kAppId1, preferred_apps_.FindPreferredAppForUrl(url_http));
+
+  preferred_apps_.DeletePreferredApp(kAppId1, intent_filter);
+  EXPECT_EQ(base::nullopt, preferred_apps_.FindPreferredAppForUrl(url_https));
+  EXPECT_EQ(base::nullopt, preferred_apps_.FindPreferredAppForUrl(url_http));
+}
+
+// Test for more than one pattern available, we can delete the filter.
+TEST_F(PreferredAppTest, DeleteDifferentPatterns) {
+  preferred_apps_.Init(
+      std::make_unique<base::Value>(base::Value::Type::DICTIONARY));
+
+  auto intent_filter_literal =
+      CreatePatternFilter("/bc", apps::mojom::PatternMatchType::kLiteral);
+  auto intent_filter_prefix =
+      CreatePatternFilter("/a", apps::mojom::PatternMatchType::kPrefix);
+  auto intent_filter_glob =
+      CreatePatternFilter("/c.*d", apps::mojom::PatternMatchType::kGlob);
+
+  preferred_apps_.AddPreferredApp(kAppId1, intent_filter_literal);
+  preferred_apps_.AddPreferredApp(kAppId2, intent_filter_prefix);
+  preferred_apps_.AddPreferredApp(kAppId3, intent_filter_glob);
+
+  GURL url_1 = GURL("https://www.google.com/bc");
+  GURL url_2 = GURL("https://www.google.com/abbb");
+  GURL url_3 = GURL("https://www.google.com/ccccccd");
+
+  EXPECT_EQ(kAppId1, preferred_apps_.FindPreferredAppForUrl(url_1));
+  EXPECT_EQ(kAppId2, preferred_apps_.FindPreferredAppForUrl(url_2));
+  EXPECT_EQ(kAppId3, preferred_apps_.FindPreferredAppForUrl(url_3));
+
+  preferred_apps_.DeletePreferredApp(kAppId1, intent_filter_literal);
+  EXPECT_EQ(base::nullopt, preferred_apps_.FindPreferredAppForUrl(url_1));
+  EXPECT_EQ(kAppId2, preferred_apps_.FindPreferredAppForUrl(url_2));
+  EXPECT_EQ(kAppId3, preferred_apps_.FindPreferredAppForUrl(url_3));
+  preferred_apps_.DeletePreferredApp(kAppId2, intent_filter_prefix);
+  EXPECT_EQ(base::nullopt, preferred_apps_.FindPreferredAppForUrl(url_2));
+  EXPECT_EQ(kAppId3, preferred_apps_.FindPreferredAppForUrl(url_3));
+  preferred_apps_.DeletePreferredApp(kAppId3, intent_filter_glob);
+  EXPECT_EQ(base::nullopt, preferred_apps_.FindPreferredAppForUrl(url_3));
+}
+
+// Test that can delete properly for super set filters. E.g. the filter
+// to delete has more condition values compare with filter that was set.
+TEST_F(PreferredAppTest, DeleteForNotCompletedFilter) {
+  preferred_apps_.Init(
+      std::make_unique<base::Value>(base::Value::Type::DICTIONARY));
+
+  auto intent_filter_set =
+      apps_util::CreateIntentFilterForUrlScope(GURL("https://www.google.com/"));
+
+  auto intent_filter_to_delete =
+      apps_util::CreateIntentFilterForUrlScope(GURL("http://www.google.com/"));
+  intent_filter_to_delete->conditions[0]->condition_values.push_back(
+      apps_util::MakeConditionValue("https",
+                                    apps::mojom::PatternMatchType::kNone));
+
+  preferred_apps_.AddPreferredApp(kAppId1, intent_filter_set);
+
+  GURL url = GURL("https://www.google.com/");
+
+  EXPECT_EQ(kAppId1, preferred_apps_.FindPreferredAppForUrl(url));
+
+  preferred_apps_.DeletePreferredApp(kAppId1, intent_filter_to_delete);
+
+  EXPECT_EQ(base::nullopt, preferred_apps_.FindPreferredAppForUrl(url));
+}