Read share targets from prefstore, and filter by engagement.

chrome/browser/webshare/share_service_impl.cc

Index: chrome/browser/webshare/share_service_impl.cc
diff --git a/chrome/browser/webshare/share_service_impl.cc b/chrome/browser/webshare/share_service_impl.cc
index 7d2ddc1d20529ceb0aa3eeef01b413a88107a297..299cf198308e613f101f6c29f4679d08ad0077f0 100644
--- a/chrome/browser/webshare/share_service_impl.cc
+++ b/chrome/browser/webshare/share_service_impl.cc
@@ -10,12 +10,15 @@
 
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/engagement/site_engagement_service.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "net/base/escape.h"
 
@@ -125,28 +128,78 @@ void ShareServiceImpl::ShowPickerDialog(
 #endif
 }
 
+Browser* ShareServiceImpl::GetBrowser() {
+  return BrowserList::GetInstance()->GetLastActive();

[Sam McNally, 2017/02/09 02:10:39]

This could be the wrong Browser. Instead, ShareServiceImpl should be constructed
with a WebContents and use that to look up the Browser and get the
Profile/BrowserContext.

[constantina, 2017/02/09 05:40:11]

Okay, can I do this in another CL? This was already used before this CL, and I
just moved it into this function.

+}
+
 void ShareServiceImpl::OpenTargetURL(const GURL& target_url) {
 // TODO(constantina): Prevent this code from being run/compiled in android.
 #if defined(OS_LINUX) || defined(OS_WIN)
-  Browser* browser = BrowserList::GetInstance()->GetLastActive();
+  Browser* browser = GetBrowser();
   chrome::AddTabAt(browser, target_url,
                    browser->tab_strip_model()->active_index() + 1, true);
 #endif
 }
 
+std::string ShareServiceImpl::GetTargetTemplate(const std::string& target_url) {
+  const base::DictionaryValue* share_target_dict =
+      GetPrefService()->GetDictionary(prefs::kWebShareVisitedTargets);
+
+  const base::DictionaryValue* share_target_info_dict = nullptr;
+  share_target_dict->GetDictionaryWithoutPathExpansion(target_url,

[Sam McNally, 2017/02/09 02:10:39]

Can this fail? Could a share target be removed while the picker is shown?

[constantina, 2017/02/09 05:40:10]

Yes, it can, if the site's manifest is changed and redownloaded between pref
reads. I've now created a copy of the target prefs when I first read them, and
stored the copy as a member variable. The URL may not be handled by the target
site, if it's no longer a target, but there is no guarantee on that if it is a
target anyway (could declare template but not handle it).

+                                                       &share_target_info_dict);
+
+  std::string url_template;
+  share_target_info_dict->GetString("url_template", &url_template);
+  return url_template;
+}
+
+PrefService* ShareServiceImpl::GetPrefService() {
+  return GetBrowser()->profile()->GetPrefs();
+}
+
+blink::mojom::EngagementLevel ShareServiceImpl::GetEngagementLevel(
+    const GURL& url) {
+  SiteEngagementService* site_engagement_service =
+      SiteEngagementService::Get(GetBrowser()->profile());
+  return site_engagement_service->GetEngagementLevel(url);
+}
+
+// static
+std::vector<std::pair<base::string16, GURL>>
+ShareServiceImpl::GetTargetsWithSufficientEngagement() {
+  const blink::mojom::EngagementLevel kMinimumEngagementLevel =

[Sam McNally, 2017/02/09 02:10:39]

constexpr

[constantina, 2017/02/09 05:40:10]

Done.

+      blink::mojom::EngagementLevel::LOW;
+
+  const base::DictionaryValue* dict =
+      GetPrefService()->GetDictionary(prefs::kWebShareVisitedTargets);
+
+  std::vector<std::pair<base::string16, GURL>> sufficiently_engaged_targets;
+
+  for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
+    GURL manifest_url(it.key());
+    if (GetEngagementLevel(manifest_url) >= kMinimumEngagementLevel) {
+      const base::DictionaryValue* share_target_dict;
+      bool result = it.value().GetAsDictionary(&share_target_dict);
+      DCHECK(result);
+
+      std::string name;
+      share_target_dict->GetString("name", &name);
+
+      sufficiently_engaged_targets.push_back(
+          make_pair(base::UTF8ToUTF16(name), manifest_url));
+    }
+  }
+
+  return sufficiently_engaged_targets;
+}
+
 void ShareServiceImpl::Share(const std::string& title,
                              const std::string& text,
                              const GURL& share_url,
                              const ShareCallback& callback) {
-  // TODO(constantina): Replace hard-coded name and manifest URL with the list
-  // of registered targets' manifest URLs.
-  constexpr char kTargetName[] = "Web Share Target Test App";
-  constexpr char kManifestURL[] =
-      "https://wicg.github.io/web-share-target/demos/manifest.json";
-  // TODO(constantina): Pass vector of pairs of target names and manifest URLs
-  // to picker.
-  std::vector<std::pair<base::string16, GURL>> targets{make_pair(
-      base::ASCIIToUTF16(kTargetName), GURL(kManifestURL))};
+  std::vector<std::pair<base::string16, GURL>> targets =
+      GetTargetsWithSufficientEngagement();
 
   ShowPickerDialog(targets, base::Bind(&ShareServiceImpl::OnPickerClosed,
                                        base::Unretained(this), title, text,
@@ -163,27 +216,29 @@ void ShareServiceImpl::OnPickerClosed(const std::string& title,
     return;
   }
 
-  // TODO(constantina): use manifest URL in result to look up corresponding URL
-  // template.
-  constexpr char kUrlTemplate[] =
-      "https://wicg.github.io/web-share-target/demos/"
-      "sharetarget.html?title={title}&text={text}&url={url}";
+  std::string chosen_target = result.value();
 
+  std::string url_template = GetTargetTemplate(chosen_target);
   std::string url_template_filled;
-  if (!ReplacePlaceholders(kUrlTemplate, title, text, share_url,
-                           &url_template_filled)) {
+  if (!ReplacePlaceholders(url_template, title, text, share_url,
+                          &url_template_filled)) {
     callback.Run(base::Optional<std::string>(
         "Error: unable to replace placeholders in url template"));
     return;
   }
 
-  GURL target_url(url_template_filled);
-  if (!target_url.is_valid()) {
+  // The template is relative to the manifest URL (minus the filename).
+  // Concatenate to make an absolute URL.
+  base::StringPiece url_base(
+      chosen_target.data(),
+      chosen_target.size() - GURL(chosen_target).ExtractFileName().size());
+  const GURL target(url_base.as_string() + url_template_filled);
+  if (!target.is_valid()) {
     callback.Run(base::Optional<std::string>(
         "Error: url of share target is not a valid url."));
     return;
   }
-  OpenTargetURL(target_url);
+  OpenTargetURL(target);
 
   callback.Run(base::nullopt);
 }