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 94f7b90bbac82a5778364337fa43c69e64bff108..ba766f56e3f1dba1dd9ade4459bd35bd098f22e7 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,22 +128,71 @@ void ShareServiceImpl::ShowPickerDialog( |
#endif |
} |
+Browser* ShareServiceImpl::GetBrowser() { |
+ return BrowserList::GetInstance()->GetLastActive(); |
+} |
+ |
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(); |
- chrome::AddTabAt(browser, target_url, |
- browser->tab_strip_model()->active_index() + 1, true); |
+ chrome::AddTabAt(GetBrowser(), target_url, |
Matt Giuca
2017/02/01 07:30:20
You should still assign this to a Browser* local v
constantina
2017/02/02 00:43:47
Done.
|
+ GetBrowser()->tab_strip_model()->active_index() + 1, true); |
#endif |
} |
+std::string ShareServiceImpl::GetTargetTemplate(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, |
+ &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(GURL url) { |
+ SiteEngagementService* site_engagement_service = |
+ SiteEngagementService::Get(GetBrowser()->profile()); |
+ return site_engagement_service->GetEngagementLevel(url); |
+} |
+ |
+// static |
+std::vector<std::string> |
+ShareServiceImpl::GetTargetsWithSufficientEngagement() { |
+ const base::DictionaryValue* dict = |
+ GetPrefService()->GetDictionary(prefs::kWebShareVisitedTargets); |
+ |
+ std::vector<std::string> most_engaged_targets; |
Matt Giuca
2017/02/01 07:30:20
Not really the right name (it implies that you'll
constantina
2017/02/02 00:43:47
Done.
|
+ |
+ for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) { |
Matt Giuca
2017/02/01 07:30:20
auto?
Also, what a strange iterator protocol!
constantina
2017/02/02 00:43:48
Compiler cries.
Ikr!
Matt Giuca
2017/02/02 02:57:10
Oh, right, you can't call auto because you're actu
|
+ GURL manifest_url(it.key()); |
Matt Giuca
2017/02/01 07:30:20
This can probably be a const GURL& yes?
constantina
2017/02/02 00:43:47
Done. Just const, no "&". The key is a string, not
Matt Giuca
2017/02/02 02:57:10
Oh OK, then could just remove the const as well (I
constantina
2017/02/02 05:57:07
Removed
|
+ if (GetEngagementLevel(manifest_url) >= |
+ blink::mojom::EngagementLevel::LOW) { |
Matt Giuca
2017/02/01 07:30:20
Can this be a constant (e.g., kMinimumEngagementLe
constantina
2017/02/02 00:43:47
I moved it to the top of the function.
Matt Giuca
2017/02/02 02:57:10
Acknowledged.
|
+ most_engaged_targets.push_back(it.key()); |
+ } |
+ } |
+ |
+ return most_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 with the registered target list. |
- constexpr char kUrlBase[] = "https://wicg.github.io/web-share-target/demos/"; |
- std::vector<base::string16> targets{base::ASCIIToUTF16(kUrlBase)}; |
+ std::vector<std::string> most_engaged_targets = |
Matt Giuca
2017/02/01 07:30:20
Same name as previous function.
constantina
2017/02/02 00:43:47
Done.
|
+ GetTargetsWithSufficientEngagement(); |
+ |
+ std::vector<base::string16> targets; |
+ for (const std::string& target : most_engaged_targets) { |
+ targets.push_back(base::ASCIIToUTF16(target)); |
Matt Giuca
2017/02/01 07:30:20
UTF8ToUTF16
You should never use ASCIIToUTF16 unl
constantina
2017/02/02 00:43:47
Done.
|
+ } |
ShowPickerDialog(targets, base::Bind(&ShareServiceImpl::OnPickerClosed, |
base::Unretained(this), title, text, |
@@ -157,25 +209,29 @@ void ShareServiceImpl::OnPickerClosed(const std::string& title, |
return; |
} |
- constexpr char kUrlTemplate[] = |
- "sharetarget.html?title={title}&text={text}&url={url}"; |
+ std::string chosen_target = base::UTF16ToASCII(result.value()); |
Matt Giuca
2017/02/01 07:30:20
UTF16ToUTF8
constantina
2017/02/02 00:43:48
Done.
|
+ std::string url_template = GetTargetTemplate(chosen_target); |
Matt Giuca
2017/02/01 07:30:20
I'm a big fan of this approach versus what we disc
constantina
2017/02/02 00:43:47
:D It has turned out to be quite neat; I like that
|
std::string url_template_filled; |
- if (!ReplacePlaceholders(kUrlTemplate, title, text, share_url, |
- &url_template_filled)) { |
- callback.Run(base::Optional<std::string>( |
- "Error: unable to replace placeholders in url template")); |
+ if (!ReplacePlaceholders(url_template, title, text, share_url, |
+ &url_template_filled)) { |
+ callback.Run( |
+ base::Optional<std::string>("Error: placeholders in share target's url " |
Matt Giuca
2017/02/01 07:30:20
Is there a reason to change this string?
constantina
2017/02/02 00:43:48
No, I just lost it in a rebase, wrote something si
|
+ "template could not be replaced.")); |
return; |
} |
- std::string url_base = base::UTF16ToASCII(result.value()); |
- GURL target_url(url_base + url_template_filled); |
- if (!target_url.is_valid()) { |
- callback.Run(base::Optional<std::string>( |
- "Error: url of share target is not a valid url.")); |
+ base::StringPiece url_base( |
Matt Giuca
2017/02/01 07:30:20
// The template is relative to the manifest URL (m
constantina
2017/02/02 00:43:47
Done.
|
+ 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: chosen share target's url template " |
+ "does not result in a valid url.")); |
return; |
} |
- OpenTargetURL(target_url); |
+ OpenTargetURL(target); |
callback.Run(base::nullopt); |
} |