Implemented stub ShareService mojo service, for navigator.share.

chrome/browser/webshare/share_service_impl_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/run_loop.h"
+#include "chrome/browser/webshare/share_service_impl.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+// Tests the Share Service mojo service defined in
+// chrome/browser/webshare/share_service_impl.h

[Matt Giuca, 2016/12/08 06:42:56]

No comment required here.

[constantina, 2016/12/09 02:24:15]

Done.

+class ShareServiceTest : public ChromeRenderViewHostTestHarness {
+ public:
+  ShareServiceTest() {}
+  ~ShareServiceTest() override {}
+
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();

[Matt Giuca, 2016/12/08 06:42:56]

nit: Blank line after.

[constantina, 2016/12/09 02:24:14]

Done.

+    ShareServiceImpl::Create(mojo::GetProxy(&share_service_));
+  }
+
+  void callback(const base::Optional<std::string>& str) {

[Matt Giuca, 2016/12/08 06:42:56]

Add an argument "const base::Optional<std::string>& expected" which you pass in
to base::Bind below.

Because each test will expect a different result (potentially), so you want the
expected value (either null or a string) to be in the test, not in the class
definition.

[constantina, 2016/12/09 02:24:15]

Done.

+    EXPECT_EQ(base::Optional<std::string>("Not implemented: navigator.share"),
+              str);
+
+    if (!quit_closure_.is_null())
+      quit_closure_.Run();
+  }
+
+  blink::mojom::ShareServicePtr share_service_;
+  base::Closure quit_closure_;
+};
+
+// Basic test to check the Share method uses the callback as expected.
+TEST_F(ShareServiceTest, ShareCallbackSuccess) {
+  LOG(ERROR) << "ShareCallbackSuccess entered";

[Matt Giuca, 2016/12/08 06:42:56]

Remove this debug log.

[constantina, 2016/12/09 02:24:14]

Done.

+  const GURL url("https://www.google.com");
+
+  base::RunLoop run_loop;

[Matt Giuca, 2016/12/08 06:42:56]

Move this into SetUp (and make run_loop a class variable so it can be Run() from
the test).

[constantina, 2016/12/09 02:24:14]

Can't be done easily. In addition, run_loop can't be elegantly used multiple
times in the same test.

[Matt Giuca, 2016/12/09 02:32:21]

Acknowledged.

+  quit_closure_ = run_loop.QuitClosure();
+
+  base::Callback<void(const base::Optional<std::string>&)> cb =

[Matt Giuca, 2016/12/08 06:42:56]

style: Don't abbreviate words in variable names (cb -> callback).

[constantina, 2016/12/09 02:24:14]

Done.

+      base::Bind(&ShareServiceTest::callback, base::Unretained(this));
+  share_service_->Share("title", "text", url, cb);
+
+  run_loop.Run();

[Matt Giuca, 2016/12/08 06:42:56]

You *might* be able to get away with putting this in TearDown() instead of
having to put it at the end of each test. Try it and see if it works (and fails
correctly too).

(As I said, it might be wrong to do that since technically the callback checking
will happen in the TearDown, but I think that might be preferable if it works,
to having to call Run() at the end of every test.)

[constantina, 2016/12/09 02:24:15]

Acknowledged.

+}