Default share to Share Target: Partial impl. of Web Share for Desktop.

chrome/browser/webshare/share_service_impl.h

Index: chrome/browser/webshare/share_service_impl.h
diff --git a/chrome/browser/webshare/share_service_impl.h b/chrome/browser/webshare/share_service_impl.h
index 83de10f9511ff36394283b8f200f331ae0dad769..f9914f7ebb3ea8a885867b625c4eb864b114b708 100644
--- a/chrome/browser/webshare/share_service_impl.h
+++ b/chrome/browser/webshare/share_service_impl.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/gtest_prod_util.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "third_party/WebKit/public/platform/modules/webshare/webshare.mojom.h"
 
@@ -20,12 +21,34 @@ class ShareServiceImpl : public blink::mojom::ShareService {
 
   static void Create(mojo::InterfaceRequest<ShareService> request);
 
+  // blink::mojom::ShareService overrides:
   void Share(const std::string& title,
              const std::string& text,
-             const GURL& url,
+             const GURL& share_url,
              const ShareCallback& callback) override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(ShareServiceImplUnittest, ReplacePlaceholders);
+
+  // Opens a new tab and navigates to |target_url|.
+  // Virtual for testing purposes.
+  virtual void OpenTargetURL(const GURL& target_url);
+
+  // Writes to |url_template_filled|, a copy of |url_template with all instances

[Matt Giuca, 2017/01/05 03:09:25]

nit: "|url_template" needs a close '|'.

[constantina, 2017/01/09 02:12:43]

Done.

+  // of "%{title}", "%{text}", and "%{url}" (quotes for clarity) replaced with

[Matt Giuca, 2017/01/05 03:09:25]

Drop "(quotes for clarity)".

[constantina, 2017/01/09 02:12:43]

Done.

+  // |title|, |text|, and |url| respectively.
+  // Replaces instances of "%{other}" where other is any string besides title,

[Matt Giuca, 2017/01/05 03:09:25]

Quote "other", "title", "text" and "url".

Also maybe replace "other" with "X".

[constantina, 2017/01/09 02:12:43]

Done.

+  // text, and url, with and empty string, for backwards compatibility.

[Matt Giuca, 2017/01/05 03:09:26]

with *an* empty string.

For *forwards* compatibility (this is done so we don't break if we add a new one
in the future, not to support a legacy thing that's been removed).

[constantina, 2017/01/09 02:12:43]

Done.

+  // Returns 1, and writes the untouched |url_template| to
+  // |url_template_filled|, if there are badly nested placeholders.
+  // This includes any case in which two "%{" occur before a "}", or a "}"
+  // occurs with no preceding "%{".
+  static int ReplacePlaceholders(const std::string& url_template,

[Matt Giuca, 2017/01/05 03:09:25]

Can this return a bool, not an int?

[constantina, 2017/01/09 02:12:43]

Done.

+                                 const std::string& title,

[Matt Giuca, 2017/01/05 03:09:26]

What happened to StringPiece?

[constantina, 2017/01/09 02:12:43]

Sam said to use std::string& if it gets passed directly to something that needs
a string, which it does.

[Sam McNally, 2017/01/09 03:15:21]

It doesn't anymore.

[Matt Giuca, 2017/01/09 03:56:54]

None of these do though.

url_template gets inspected by SplitTemplate, which never stores it in a string,
so it could take a StringPiece and never copy the string.

title and text get passed to net::EscapeQueryParamValue which takes a
StringPiece.

[constantina, 2017/01/09 23:51:03]

Done.

+                                 const std::string& text,
+                                 const GURL& share_url,
+                                 std::string* url_template_filled);
+
   DISALLOW_COPY_AND_ASSIGN(ShareServiceImpl);
 };