Implement clipboardRead/Write content capabilities

chrome/browser/extensions/content_capabilities_browsertest.cc

Index: chrome/browser/extensions/content_capabilities_browsertest.cc
diff --git a/chrome/browser/extensions/content_capabilities_browsertest.cc b/chrome/browser/extensions/content_capabilities_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d1461be3071ea09b389aa1f4345ebe02255024ae
--- /dev/null
+++ b/chrome/browser/extensions/content_capabilities_browsertest.cc
@@ -0,0 +1,164 @@
+// Copyright 2014 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 <string>
+#include <vector>
+
+#include "base/command_line.h"
+#include "base/path_service.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/extensions/test_extension_dir.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "components/crx_file/id_util.h"
+#include "content/public/test/browser_test_utils.h"
+#include "extensions/common/extension_builder.h"
+#include "extensions/common/manifest_handlers/content_capabilities_handler.h"
+#include "extensions/common/switches.h"
+#include "extensions/common/url_pattern.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+
+using extensions::DictionaryBuilder;
+using extensions::Extension;
+using extensions::ExtensionBuilder;
+using extensions::ListBuilder;
+
+class ContentCapabilitiesTest : public ExtensionApiTest {
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    ExtensionApiTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(
+        extensions::switches::kWhitelistedExtensionID,
+        crx_file::id_util::GenerateIdForPath(
+            test_extension_dir_.unpacked_path()));
+  }
+
+  // Builds an extension manifest with the given content_capabilities matches
+  // and permissions. The extension always has the same (whitelisted) ID.
+  scoped_refptr<const Extension> LoadExtensionWithCapabilities(
+      const std::string& matches,
+      const std::string& permissions) {
+    std::string manifest = base::StringPrintf(
+        "{\n"
+        "  \"name\": \"content_capabilities test extensions\",\n"
+        "  \"version\": \"1\",\n"
+        "  \"manifest_version\": 2,\n"
+        "  \"content_capabilities\": {\n"
+        "    \"matches\": [%s],\n"
+        "    \"permissions\": [%s]\n"
+        "  }\n"
+        "}\n",
+        matches.c_str(),
+        permissions.c_str());
+    test_extension_dir_.WriteManifest(manifest);
+    return LoadExtension(test_extension_dir_.unpacked_path());
+  }
+
+  std::string MakeJSONList(
+      const std::string& s0 = "",
+      const std::string& s1 = "",
+      const std::string& s2 = "") {
+    std::vector<std::string> v;
+    if (!s0.empty()) v.push_back(s0);
+    if (!s1.empty()) v.push_back(s1);
+    if (!s2.empty()) v.push_back(s2);
+    std::string list = JoinString(v, "\",\"");
+    if (!list.empty())
+      list = "\"" + list + "\"";

[not at google - send to devlin, 2014/12/10 17:33:54]

This is borderline too nitty for me, but I'll say it anyway - a JSON list would
be ["a", "b", "c"] not "a", "b", "c". So consider doing that, then not wrapping
it in the [] as part of LoadExtensionWithCapabilities.

Also, subtle bug which exists right now is that MakeJSONList() will return an
empty string, which would end up above having a singleton [""] list above,
rather than the empty list. The best way to fix this, I think, would be to make
the change I suggest here.

[Ken Rockot(use gerrit already), 2014/12/10 18:31:41]

Actually an empty string will lead to the list [] (consider:
StringPrintf("[%s]", ""))

But I agree it's semantically stupid that MakeJSONList doesn't return a JSON
list, so I've changed it anyway.

+    return list;
+  }
+
+  content::WebContents* web_contents() {
+    return browser()->tab_strip_model()->GetActiveWebContents();
+  }
+
+  GURL GetTestURLFor(const std::string& host) {
+    std::string port = base::IntToString(embedded_test_server()->port());
+    GURL::Replacements replacements;
+    replacements.SetHostStr(host);
+    replacements.SetPortStr(port);
+    return embedded_test_server()->GetURL("/" + host + ".html")
+        .ReplaceComponents(replacements);
+  }
+
+  void InitializeTestServer() {
+    base::FilePath test_data;
+    EXPECT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data));
+    embedded_test_server()->ServeFilesFromDirectory(test_data.AppendASCII(
+        "extensions/content_capabilities"));
+    ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+    host_resolver()->AddRule("*", embedded_test_server()->base_url().host());
+  }
+
+ private:
+  extensions::TestExtensionDir test_extension_dir_;
+};
+
+IN_PROC_BROWSER_TEST_F(ContentCapabilitiesTest, ClipboardRead) {
+  InitializeTestServer();
+  scoped_refptr<const Extension> extension = LoadExtensionWithCapabilities(
+      MakeJSONList("http://foo.example.com/*"),
+      MakeJSONList("clipboardRead"));

[not at google - send to devlin, 2014/12/10 17:33:54]

Is it worth testing an empty set of permissions?

[Ken Rockot(use gerrit already), 2014/12/10 18:31:41]

Sure.

+  ui_test_utils::NavigateToURL(browser(), GetTestURLFor("foo.example.com"));
+  bool result = false;
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canReadClipboard()", &result));
+  EXPECT_TRUE(result);
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canWriteClipboard()", &result));
+  EXPECT_FALSE(result);
+
+  ui_test_utils::NavigateToURL(browser(), GetTestURLFor("bar.example.com"));
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canReadClipboard()", &result));
+  EXPECT_FALSE(result);
+}
+
+IN_PROC_BROWSER_TEST_F(ContentCapabilitiesTest, ClipboardWrite) {
+  InitializeTestServer();
+  scoped_refptr<const Extension> extension = LoadExtensionWithCapabilities(
+      MakeJSONList("http://foo.example.com/*"),
+      MakeJSONList("clipboardWrite"));
+  ui_test_utils::NavigateToURL(browser(), GetTestURLFor("foo.example.com"));
+  bool result = false;
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canWriteClipboard()", &result));
+  EXPECT_TRUE(result);
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canReadClipboard()", &result));
+  EXPECT_FALSE(result);
+
+  ui_test_utils::NavigateToURL(browser(), GetTestURLFor("bar.example.com"));
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canWriteClipboard()", &result));
+  EXPECT_FALSE(result);
+}
+
+IN_PROC_BROWSER_TEST_F(ContentCapabilitiesTest, ClipboardReadWrite) {
+  InitializeTestServer();
+  scoped_refptr<const Extension> extension = LoadExtensionWithCapabilities(
+      MakeJSONList("http://foo.example.com/*"),
+      MakeJSONList("clipboardRead", "clipboardWrite"));
+  ui_test_utils::NavigateToURL(browser(), GetTestURLFor("foo.example.com"));
+  bool result = false;
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canWriteClipboard()", &result));
+  EXPECT_TRUE(result);
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canReadClipboard()", &result));
+  EXPECT_TRUE(result);

[not at google - send to devlin, 2014/12/10 17:33:54]

You should be able to cut out a bunch of this boilerplate by writing a function
that returns a testing::AssertionResult, like,

testing::AssertionResult TestClipboardAccess(
    const Extension* extension, const char* origin,
    bool can_read, bool can_write) {
  ui_test_utils::NavigateToURL(...);
  bool result = false;
  CHECK(content::ExecuteScriptAndExtractBool(...));
  if (result != can_read)
    return testing::AssertionFailure() << ...;
  ...
  return testing::AssertionSuccess();
}

then

InitializeTestServer();
scoped_refptr<const Extension> extension = ...;
EXPECT_TRUE(TestClipboardAccess(...));

[Ken Rockot(use gerrit already), 2014/12/10 18:31:41]

Good call, though I don't really see the value in using TestAssertion over
bools. The result should still be meaningful test failure output (i.e. line
number, excerpt including the name e.g. "CanReadClipboard"), shouldn't it?

[not at google - send to devlin, 2014/12/10 19:30:07]

The value in testing::AssertionResult is that it maintains stack trace, and it
preserves any nested << you put inside that helper function.

It actually doesn't make a difference the way you have it now, but if you were
to go with the hybrid method that I wrote up above, take for example:

Say that clipboardRead works but clipboardWrite doesn't - it always return true,
or maybe it always returns false. In the always-returns-true case, the
ClipboardReadWrite test would fail - "expected true, got false". That's not very
good. What you want to see it "expected success", with the stack trace, and if
you streamed out "incorrect value for write" then you'd see that too.

This is super useful when you have more complex helper methods. Smaller methods
less so, but I just think that in the spirit of using the same patterns
everywhere, helper test methods should always return testing::AssertionResults.

Also, I preferred that hybrid way :) but NBD.

+
+  ui_test_utils::NavigateToURL(browser(), GetTestURLFor("bar.example.com"));
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canWriteClipboard()", &result));
+  EXPECT_FALSE(result);
+  CHECK(content::ExecuteScriptAndExtractBool(web_contents(),
+     "tests.canReadClipboard()", &result));
+  EXPECT_FALSE(result);
+}