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..0061d94581c17403f2c5ba6a179039134a0263ce 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,40 @@ 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);
+
+  // Splits |url_template| on %{ and }, and stores in |split_template|.
+  // Ensures elements at even indices of |split_template| are the substrings
+  // between %{ and } in the |url_template|. Thus, if } was immediately followed

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

nit: Add quotes around the literal bits to make it easier to read ("%{" and "}",
etc). (You have done this elsewhere, it seems.)

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

Done.

+  // by %{, an empty string is stored.

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

Document the return conditions.

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

Done.

+  static bool SplitTemplate(const std::string& url_template,

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

Maybe a quick example to clarify:

Input: "abc%{def}%{ghi}"
Output: {"abc", "def", "", "ghi", ""}.

Also is this always guaranteed to have an odd number of output strings? (i.e. if
there is nothing before the first template, or nothing after the last template,
will it generate an empty string?) If not, I think it should. Document this.

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

This doesn't have to be a static method; it can just be in an unnamed namespace
in the .cc file (without "static").

See
https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Stat...

(I was going to say the same thing about ReplacePlaceholders but I realised it
needs to be called by the test.)

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

Done.

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

Done.

+                            std::vector<base::StringPiece>& split_template);
+
+  // Writes to |url_template_filled|, a copy of |url_template| with all
+  // instances of "%{title}", "%{text}", and "%{url}" replaced with
+  // |title|, |text|, and |url| respectively.
+  // Replaces instances of "%{X}" where "X" is any string besides "title",
+  // "text", and "url", with an empty string, for forwards compatibility.
+  // Returns true, if there are badly nested placeholders.
+  // This includes any case in which two "%{" occur before a "}", or a "}"
+  // occurs with no preceding "%{".
+  static bool ReplacePlaceholders(const std::string& url_template,
+                                 const std::string& title,
+                                 const std::string& text,
+                                 const GURL& share_url,
+                                 std::string* url_template_filled);
+
   DISALLOW_COPY_AND_ASSIGN(ShareServiceImpl);
 };