[IntentHandling] Store and read preferred app from disk.

This CL stores and reads the preferred app from disk. This CL also
updates the intent picker bubble view disable checkout box code to
allow testing with PWA.

BUG=853604

Change-Id: I675f09281a27ddd9e11ab3d28389101445c02d68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902177
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713738}

chrome/browser/ui/views/intent_picker_bubble_view.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/feature_list.h"
 #include "base/i18n/rtl.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
@@ -16,6 +17,7 @@
 #include "chrome/browser/ui/views/chrome_typography.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/url_formatter/elide_url.h"
 #include "content/public/browser/navigation_handle.h"
@@ -514,11 +516,14 @@ void IntentPickerBubbleView::UpdateCheckboxState() {
   // TODO(crbug.com/826982): allow PWAs to have their decision persisted when
   // there is a central Chrome OS apps registry to store persistence.
   // TODO(crbug.com/1000037): allow to persist remote devices too.
-  auto selected_app_type = app_info_[selected_app_tag_].type;
-  const bool should_enable =
-      selected_app_type != apps::PickerEntryType::kWeb &&
-      selected_app_type != apps::PickerEntryType::kDevice;
-
+  bool should_enable = false;
+  if (base::FeatureList::IsEnabled(features::kAppServiceIntentHandling)) {
+    should_enable = true;
+  } else {
+    auto selected_app_type = app_info_[selected_app_tag_].type;
+    should_enable = selected_app_type != apps::PickerEntryType::kWeb &&
+                    selected_app_type != apps::PickerEntryType::kDevice;
+  }
   // Reset the checkbox state to the default unchecked if becomes disabled.
   if (!should_enable)
     remember_selection_checkbox_->SetChecked(false);

chrome/services/app_service/app_service_impl.cc

@@ -11,6 +11,7 @@
 #include "chrome/services/app_service/public/mojom/types.mojom.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
 #include "services/preferences/public/cpp/pref_service_factory.h"
 #include "services/service_manager/public/cpp/connector.h"
 
@@ -34,7 +35,6 @@ AppServiceImpl::AppServiceImpl(service_manager::Connector* connector) {
   if (connector) {
     ConnectToPrefService(connector);
   }
-  InitializePreferredApps();
 }
 
 AppServiceImpl::~AppServiceImpl() = default;
@@ -81,7 +81,9 @@ void AppServiceImpl::RegisterSubscriber(
   // TODO: store the opts somewhere.
 
   // Initialise the Preferred Apps in the Subscribers on register.
-  subscriber->InitializePreferredApps(preferred_apps_.GetValue());
+  if (preferred_apps_.IsInitialized()) {
+    subscriber->InitializePreferredApps(preferred_apps_.GetValue());
+  }
 
   // Add the new subscriber to the set.
   subscribers_.Add(std::move(subscriber));
@@ -173,7 +175,20 @@ void AppServiceImpl::AddPreferredApp(apps::mojom::AppType app_type,
                                      const std::string& app_id,
                                      apps::mojom::IntentFilterPtr intent_filter,
                                      apps::mojom::IntentPtr intent) {
+  // TODO(crbug.com/853604): Solve the issue where user set preferred app
+  // before pref connected.
+  if (!preferred_apps_.IsInitialized()) {
+    return;
+  }
+
   preferred_apps_.AddPreferredApp(app_id, intent_filter);
+
+  if (pref_service_) {
+    DictionaryPrefUpdate update(pref_service_.get(), kAppServicePreferredApps);
+    DCHECK(PreferredApps::VerifyPreferredApps(update.Get()));
+    PreferredApps::AddPreferredApp(app_id, intent_filter, update.Get());
+  }
+
   for (auto& subscriber : subscribers_) {
     subscriber->OnPreferredAppSet(app_id, intent_filter->Clone());
   }
@@ -186,17 +201,21 @@ void AppServiceImpl::AddPreferredApp(apps::mojom::AppType app_type,
                                   std::move(intent));
 }
 
-void AppServiceImpl::OnPublisherDisconnected(apps::mojom::AppType app_type) {
-  publishers_.erase(app_type);
-}
-
 PreferredApps& AppServiceImpl::GetPreferredAppsForTesting() {
   return preferred_apps_;
 }
 
+void AppServiceImpl::OnPublisherDisconnected(apps::mojom::AppType app_type) {
+  publishers_.erase(app_type);
+}
+
 void AppServiceImpl::InitializePreferredApps() {
-  // TODO(crbug.com/853604): Initialise from disk.
-  preferred_apps_.Init(nullptr);
+  DCHECK(pref_service_);
+  preferred_apps_.Init(
+      pref_service_->GetDictionary(kAppServicePreferredApps)->CreateDeepCopy());
+  for (auto& subscriber : subscribers_) {
+    subscriber->InitializePreferredApps(preferred_apps_.GetValue());
+  }
 }
 
 void AppServiceImpl::ConnectToPrefService(
@@ -218,12 +237,13 @@ void AppServiceImpl::ConnectToPrefService(
 
 void AppServiceImpl::OnPrefServiceConnected(
     std::unique_ptr<PrefService> pref_service) {
-  if (!pref_service)
+  if (!pref_service) {
+    // TODO(crbug.com/853604): Handle if not successfully connected.
     return;
+  }
   DCHECK_EQ(nullptr, pref_service_);
   pref_service_ = std::move(pref_service);
-
-  // TODO(crbug.com/853604): use the pref service to get and set preference.
+  InitializePreferredApps();
 }
 
 }  // namespace apps

chrome/services/app_service/app_service_impl_unittest.cc

@@ -253,6 +253,7 @@ TEST_F(AppServiceImplTest, PubSub) {
 TEST_F(AppServiceImplTest, PreferredApps) {
   // Test Initialize.
   AppServiceImpl impl(nullptr);
+  impl.GetPreferredAppsForTesting().Init(nullptr);
 
   // TODO(crbug.com/853604): Update this test after reading from disk done.
   EXPECT_TRUE(impl.GetPreferredAppsForTesting().GetValue().DictEmpty());

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

@@ -89,11 +89,13 @@ base::Value* FindDictionaryForTypeAndValue(
   auto* condition_type_dict =
       dict->FindKey(ConditionTypeToString(condition_type));
 
-  if (!condition_type_dict)
+  if (!condition_type_dict) {
     return nullptr;
+  }
 
-  if (condition_type != apps::mojom::ConditionType::kPattern)
+  if (condition_type != apps::mojom::ConditionType::kPattern) {
     return condition_type_dict->FindKey(value);
+  }
 
   // For pattern matching, we need to go through all patterns and match types
   // to see if we have a match.
@@ -152,11 +154,13 @@ base::Value* FindDictAndUpdateBestMatchAppId(
     base::Value* dict,
     base::Optional<std::string>* best_match_app_id) {
   auto* found_dict = FindDictionaryForTypeAndValue(dict, condition_type, value);
-  if (!found_dict)
+  if (!found_dict) {
     return found_dict;
+  }
   std::string* app_id = found_dict->FindStringKey(kAppId);
-  if (app_id)
+  if (app_id) {
     *best_match_app_id = *app_id;
+  }
   return found_dict;
 }
 
@@ -249,20 +253,22 @@ PreferredApps::~PreferredApps() = default;
 
 // static
 // Recursively verifies that the structure of |value| matches what we expect.
-
+//
 // |value| should be a dictionary where each item is either:
 // * key == kAppId and a string value, or
 // * some other string value with a dictionary value.
 bool PreferredApps::VerifyPreferredApps(base::Value* value) {
-  if (!value->is_dict())
+  if (!value->is_dict()) {
     return false;
+  }
   bool all_items_valid = true;
   for (const auto& key_value : value->DictItems()) {
     bool item_valid = false;
-    if (key_value.first == kAppId)
+    if (key_value.first == kAppId) {
       item_valid = key_value.second.is_string();
-    else
+    } else {
       item_valid = VerifyPreferredApps(&key_value.second);
+    }
     if (!item_valid) {
       all_items_valid = false;
       break;
@@ -271,6 +277,24 @@ bool PreferredApps::VerifyPreferredApps(base::Value* value) {
   return all_items_valid;
 }
 
+// static
+// Add a preferred app for a preferred app dictionary.
+bool PreferredApps::AddPreferredApp(
+    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 set
+  // preferred app for and |intent_filter|, we need to add the preferred app for
+  // all combinations of these |condition_values|.
+  SetPreferredApp(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);
@@ -286,21 +310,16 @@ bool PreferredApps::AddPreferredApp(
   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 set
-  // preferred app for and |intent_filter|, we need to add the preferred app for
-  // all combinations of these |condition_values|.
-  SetPreferredApp(intent_filter->conditions, 0, preferred_apps_.get(), app_id);
-  return true;
+  return AddPreferredApp(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;
 
-  if (!preferred_apps_)
+  if (!preferred_apps_) {
     return best_match_app_id;
+  }
 
   // Currently only support intent that has the full URL.
   if (!intent->scheme.has_value() || !intent->host.has_value() ||
@@ -314,14 +333,16 @@ base::Optional<std::string> PreferredApps::FindPreferredAppForIntent(
   auto* scheme_dict = FindDictAndUpdateBestMatchAppId(
       apps::mojom::ConditionType::kScheme, intent->scheme.value(),
       preferred_apps_.get(), &best_match_app_id);
-  if (!scheme_dict)
+  if (!scheme_dict) {
     return best_match_app_id;
+  }
 
   auto* host_dict = FindDictAndUpdateBestMatchAppId(
       apps::mojom::ConditionType::kHost, intent->host.value(), scheme_dict,
       &best_match_app_id);
-  if (!host_dict)
+  if (!host_dict) {
     return best_match_app_id;
+  }
 
   FindDictAndUpdateBestMatchAppId(apps::mojom::ConditionType::kPattern,
                                   intent->path.value(), host_dict,
@@ -339,4 +360,8 @@ base::Value PreferredApps::GetValue() {
   return preferred_apps_->Clone();
 }
 
+bool PreferredApps::IsInitialized() {
+  return preferred_apps_ != nullptr;
+}
+
 }  // namespace apps

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

@@ -45,6 +45,11 @@ class PreferredApps {
 
   static bool VerifyPreferredApps(base::Value* dict);
 
+  // Add a preferred app for an |intent_filter| for |preferred_apps|.
+  static bool AddPreferredApp(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|.
@@ -61,6 +66,8 @@ class PreferredApps {
   // Get a copy of the preferred apps.
   base::Value GetValue();
 
+  bool IsInitialized();
+
  private:
   std::unique_ptr<base::Value> preferred_apps_;