Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(135)

Side by Side Diff: chrome/browser/webshare/share_service_impl.cc

Issue 2664033002: Read share targets from prefstore, and filter by engagement. (Closed)
Patch Set: Fixed, as per feedback Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2016 The Chromium Authors. All rights reserved. 1 // Copyright 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/webshare/share_service_impl.h" 5 #include "chrome/browser/webshare/share_service_impl.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <functional> 8 #include <functional>
9 #include <utility> 9 #include <utility>
10 10
11 #include "base/strings/string_util.h" 11 #include "base/strings/string_util.h"
12 #include "base/strings/utf_string_conversions.h" 12 #include "base/strings/utf_string_conversions.h"
13 #include "chrome/browser/ui/browser.h" 13 #include "chrome/browser/engagement/site_engagement_service.h"
14 #include "chrome/browser/profiles/profile.h"
14 #include "chrome/browser/ui/browser_commands.h" 15 #include "chrome/browser/ui/browser_commands.h"
15 #include "chrome/browser/ui/browser_dialogs.h" 16 #include "chrome/browser/ui/browser_dialogs.h"
16 #include "chrome/browser/ui/browser_list.h" 17 #include "chrome/browser/ui/browser_list.h"
17 #include "chrome/browser/ui/browser_tabstrip.h" 18 #include "chrome/browser/ui/browser_tabstrip.h"
18 #include "chrome/browser/ui/tabs/tab_strip_model.h" 19 #include "chrome/browser/ui/tabs/tab_strip_model.h"
20 #include "chrome/common/pref_names.h"
21 #include "components/prefs/pref_service.h"
19 #include "mojo/public/cpp/bindings/strong_binding.h" 22 #include "mojo/public/cpp/bindings/strong_binding.h"
20 #include "net/base/escape.h" 23 #include "net/base/escape.h"
21 24
22 namespace { 25 namespace {
23 26
24 // Determines whether a character is allowed in a URL template placeholder. 27 // Determines whether a character is allowed in a URL template placeholder.
25 bool IsIdentifier(char c) { 28 bool IsIdentifier(char c) {
26 return base::IsAsciiAlpha(c) || base::IsAsciiDigit(c) || c == '-' || c == '_'; 29 return base::IsAsciiAlpha(c) || base::IsAsciiDigit(c) || c == '-' || c == '_';
27 } 30 }
28 31
(...skipping 89 matching lines...) Expand 10 before | Expand all | Expand 10 after
118 const base::Callback<void(base::Optional<std::string>)>& callback) { 121 const base::Callback<void(base::Optional<std::string>)>& callback) {
119 // TODO(mgiuca): Get the browser window as |parent_window|. 122 // TODO(mgiuca): Get the browser window as |parent_window|.
120 #if defined(OS_LINUX) || defined(OS_WIN) 123 #if defined(OS_LINUX) || defined(OS_WIN)
121 chrome::ShowWebShareTargetPickerDialog(nullptr /* parent_window */, targets, 124 chrome::ShowWebShareTargetPickerDialog(nullptr /* parent_window */, targets,
122 callback); 125 callback);
123 #else 126 #else
124 callback.Run(base::nullopt); 127 callback.Run(base::nullopt);
125 #endif 128 #endif
126 } 129 }
127 130
131 Browser* ShareServiceImpl::GetBrowser() {
132 return BrowserList::GetInstance()->GetLastActive();
Sam McNally 2017/02/09 02:10:39 This could be the wrong Browser. Instead, ShareSer
constantina 2017/02/09 05:40:11 Okay, can I do this in another CL? This was alread
133 }
134
128 void ShareServiceImpl::OpenTargetURL(const GURL& target_url) { 135 void ShareServiceImpl::OpenTargetURL(const GURL& target_url) {
129 // TODO(constantina): Prevent this code from being run/compiled in android. 136 // TODO(constantina): Prevent this code from being run/compiled in android.
130 #if defined(OS_LINUX) || defined(OS_WIN) 137 #if defined(OS_LINUX) || defined(OS_WIN)
131 Browser* browser = BrowserList::GetInstance()->GetLastActive(); 138 Browser* browser = GetBrowser();
132 chrome::AddTabAt(browser, target_url, 139 chrome::AddTabAt(browser, target_url,
133 browser->tab_strip_model()->active_index() + 1, true); 140 browser->tab_strip_model()->active_index() + 1, true);
134 #endif 141 #endif
135 } 142 }
136 143
144 std::string ShareServiceImpl::GetTargetTemplate(const std::string& target_url) {
145 const base::DictionaryValue* share_target_dict =
146 GetPrefService()->GetDictionary(prefs::kWebShareVisitedTargets);
147
148 const base::DictionaryValue* share_target_info_dict = nullptr;
149 share_target_dict->GetDictionaryWithoutPathExpansion(target_url,
Sam McNally 2017/02/09 02:10:39 Can this fail? Could a share target be removed whi
constantina 2017/02/09 05:40:10 Yes, it can, if the site's manifest is changed and
150 &share_target_info_dict);
151
152 std::string url_template;
153 share_target_info_dict->GetString("url_template", &url_template);
154 return url_template;
155 }
156
157 PrefService* ShareServiceImpl::GetPrefService() {
158 return GetBrowser()->profile()->GetPrefs();
159 }
160
161 blink::mojom::EngagementLevel ShareServiceImpl::GetEngagementLevel(
162 const GURL& url) {
163 SiteEngagementService* site_engagement_service =
164 SiteEngagementService::Get(GetBrowser()->profile());
165 return site_engagement_service->GetEngagementLevel(url);
166 }
167
168 // static
169 std::vector<std::pair<base::string16, GURL>>
170 ShareServiceImpl::GetTargetsWithSufficientEngagement() {
171 const blink::mojom::EngagementLevel kMinimumEngagementLevel =
Sam McNally 2017/02/09 02:10:39 constexpr
constantina 2017/02/09 05:40:10 Done.
172 blink::mojom::EngagementLevel::LOW;
173
174 const base::DictionaryValue* dict =
175 GetPrefService()->GetDictionary(prefs::kWebShareVisitedTargets);
176
177 std::vector<std::pair<base::string16, GURL>> sufficiently_engaged_targets;
178
179 for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
180 GURL manifest_url(it.key());
181 if (GetEngagementLevel(manifest_url) >= kMinimumEngagementLevel) {
182 const base::DictionaryValue* share_target_dict;
183 bool result = it.value().GetAsDictionary(&share_target_dict);
184 DCHECK(result);
185
186 std::string name;
187 share_target_dict->GetString("name", &name);
188
189 sufficiently_engaged_targets.push_back(
190 make_pair(base::UTF8ToUTF16(name), manifest_url));
191 }
192 }
193
194 return sufficiently_engaged_targets;
195 }
196
137 void ShareServiceImpl::Share(const std::string& title, 197 void ShareServiceImpl::Share(const std::string& title,
138 const std::string& text, 198 const std::string& text,
139 const GURL& share_url, 199 const GURL& share_url,
140 const ShareCallback& callback) { 200 const ShareCallback& callback) {
141 // TODO(constantina): Replace hard-coded name and manifest URL with the list 201 std::vector<std::pair<base::string16, GURL>> targets =
142 // of registered targets' manifest URLs. 202 GetTargetsWithSufficientEngagement();
143 constexpr char kTargetName[] = "Web Share Target Test App";
144 constexpr char kManifestURL[] =
145 "https://wicg.github.io/web-share-target/demos/manifest.json";
146 // TODO(constantina): Pass vector of pairs of target names and manifest URLs
147 // to picker.
148 std::vector<std::pair<base::string16, GURL>> targets{make_pair(
149 base::ASCIIToUTF16(kTargetName), GURL(kManifestURL))};
150 203
151 ShowPickerDialog(targets, base::Bind(&ShareServiceImpl::OnPickerClosed, 204 ShowPickerDialog(targets, base::Bind(&ShareServiceImpl::OnPickerClosed,
152 base::Unretained(this), title, text, 205 base::Unretained(this), title, text,
153 share_url, callback)); 206 share_url, callback));
154 } 207 }
155 208
156 void ShareServiceImpl::OnPickerClosed(const std::string& title, 209 void ShareServiceImpl::OnPickerClosed(const std::string& title,
157 const std::string& text, 210 const std::string& text,
158 const GURL& share_url, 211 const GURL& share_url,
159 const ShareCallback& callback, 212 const ShareCallback& callback,
160 base::Optional<std::string> result) { 213 base::Optional<std::string> result) {
161 if (!result.has_value()) { 214 if (!result.has_value()) {
162 callback.Run(base::Optional<std::string>("Share was cancelled")); 215 callback.Run(base::Optional<std::string>("Share was cancelled"));
163 return; 216 return;
164 } 217 }
165 218
166 // TODO(constantina): use manifest URL in result to look up corresponding URL 219 std::string chosen_target = result.value();
167 // template.
168 constexpr char kUrlTemplate[] =
169 "https://wicg.github.io/web-share-target/demos/"
170 "sharetarget.html?title={title}&text={text}&url={url}";
171 220
221 std::string url_template = GetTargetTemplate(chosen_target);
172 std::string url_template_filled; 222 std::string url_template_filled;
173 if (!ReplacePlaceholders(kUrlTemplate, title, text, share_url, 223 if (!ReplacePlaceholders(url_template, title, text, share_url,
174 &url_template_filled)) { 224 &url_template_filled)) {
175 callback.Run(base::Optional<std::string>( 225 callback.Run(base::Optional<std::string>(
176 "Error: unable to replace placeholders in url template")); 226 "Error: unable to replace placeholders in url template"));
177 return; 227 return;
178 } 228 }
179 229
180 GURL target_url(url_template_filled); 230 // The template is relative to the manifest URL (minus the filename).
181 if (!target_url.is_valid()) { 231 // Concatenate to make an absolute URL.
232 base::StringPiece url_base(
233 chosen_target.data(),
234 chosen_target.size() - GURL(chosen_target).ExtractFileName().size());
235 const GURL target(url_base.as_string() + url_template_filled);
236 if (!target.is_valid()) {
182 callback.Run(base::Optional<std::string>( 237 callback.Run(base::Optional<std::string>(
183 "Error: url of share target is not a valid url.")); 238 "Error: url of share target is not a valid url."));
184 return; 239 return;
185 } 240 }
186 OpenTargetURL(target_url); 241 OpenTargetURL(target);
187 242
188 callback.Run(base::nullopt); 243 callback.Run(base::nullopt);
189 } 244 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698