Add unit test for Site Settings Handler.

chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc

Index: chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc
diff --git a/chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc b/chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c8ed16222f4215bf3596acab05afcd3e68b6971c
--- /dev/null
+++ b/chrome/browser/ui/webui/settings/site_settings_handler_unittest.cc
@@ -0,0 +1,163 @@
+// 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 "chrome/browser/ui/webui/settings/site_settings_handler.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/ui/webui/site_settings_helper.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/content_settings/core/common/content_settings.h"
+#include "components/content_settings/core/common/content_settings_types.h"
+#include "content/public/browser/web_ui_data_source.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_web_ui.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace settings {
+
+class SiteSettingsHandlerTest : public testing::Test {
+ public:
+  SiteSettingsHandlerTest() {}
+
+  void SetUp() override {
+    profile_.reset(new TestingProfile());
+  }
+
+  Profile* profile() { return profile_.get(); }
+  content::TestWebUI* web_ui() { return &web_ui_; }
+
+  void ValidateDefault(bool expected_default, size_t expected_total_calls) {
+    EXPECT_EQ(expected_total_calls, web_ui()->call_data().size());
+
+    const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+    EXPECT_EQ("cr.webUIResponse", data.function_name());
+
+    std::string callback_id;
+    ASSERT_TRUE(data.arg1()->GetAsString(&callback_id));
+    EXPECT_EQ("test-callback-id", callback_id);

[michaelpg, 2016/04/01 09:08:28]

opt. nit: make the callback id a member or static

[Finnur, 2016/04/05 10:22:31]

Done.

+

[michaelpg, 2016/04/01 09:08:28]

should you check the call was successful via arg2()?

[Finnur, 2016/04/05 10:22:31]

Done.

+    bool enabled;
+    ASSERT_TRUE(data.arg3()->GetAsBoolean(&enabled));
+    EXPECT_EQ(expected_default, enabled);
+  }
+
+  void ValidateOrigin(
+      const std::string& expected_origin,
+      const std::string& expected_embedding,
+      const std::string& expected_setting,
+      const std::string& expected_source,
+      size_t expected_total_calls) {
+    EXPECT_EQ(expected_total_calls, web_ui()->call_data().size());
+
+    const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+    EXPECT_EQ("cr.webUIResponse", data.function_name());
+
+    std::string callback_id;
+    ASSERT_TRUE(data.arg1()->GetAsString(&callback_id));
+    EXPECT_EQ("test-callback-id", callback_id);
+
+    const base::ListValue* exceptions;
+    ASSERT_TRUE(data.arg3()->GetAsList(&exceptions));
+    EXPECT_EQ(1U, exceptions->GetSize());
+    const base::DictionaryValue* exception;
+    ASSERT_TRUE(exceptions->GetDictionary(0, &exception));
+    std::string origin, embedding_origin, setting, source;
+    ASSERT_TRUE(exception->GetString(site_settings::kOrigin, &origin));
+    ASSERT_EQ(expected_origin, origin);
+    ASSERT_TRUE(exception->GetString(
+        site_settings::kEmbeddingOrigin, &embedding_origin));
+    ASSERT_EQ(expected_embedding, embedding_origin);
+    ASSERT_TRUE(exception->GetString(site_settings::kSetting, &setting));
+    ASSERT_EQ(expected_setting, setting);
+    ASSERT_TRUE(exception->GetString(site_settings::kSource, &source));
+    ASSERT_EQ(expected_source, source);
+  }
+
+  void ValidateNoOrigin(size_t expected_total_calls) {
+    EXPECT_EQ(expected_total_calls, web_ui()->call_data().size());
+
+    const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
+    EXPECT_EQ("cr.webUIResponse", data.function_name());
+
+    std::string callback_id;
+    ASSERT_TRUE(data.arg1()->GetAsString(&callback_id));
+    EXPECT_EQ("test-callback-id", callback_id);
+
+    const base::ListValue* exceptions;
+    ASSERT_TRUE(data.arg3()->GetAsList(&exceptions));
+    EXPECT_EQ(0U, exceptions->GetSize());
+  }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+  scoped_ptr<TestingProfile> profile_;

[michaelpg, 2016/04/01 09:08:28]

can this just be a "TestingProfile profile_" member rather than a pointer?

[Finnur, 2016/04/05 10:22:31]

Done.

+  content::TestWebUI web_ui_;
+};
+
+TEST_F(SiteSettingsHandlerTest, GetAndSetDefault) {
+  scoped_ptr<SiteSettingsHandler> handler;

[michaelpg, 2016/04/01 09:08:28]

SiteSettingsHandler handler(profile());

[Finnur, 2016/04/05 10:22:31]

Done.

+  handler.reset(new SiteSettingsHandler(profile()));
+  handler->set_web_ui(web_ui());
+
+  ContentSettingsType type = CONTENT_SETTINGS_TYPE_NOTIFICATIONS;
+
+  // Test the JS -> C++ -> JS callback path for getting and setting defaults.
+  base::ListValue getArgs;
+  getArgs.Append(new base::StringValue("test-callback-id"));
+  getArgs.Append(new base::FundamentalValue(type));
+  handler->HandleGetDefaultValueForContentType(&getArgs);
+  ValidateDefault(true, 1U);

[Finnur, 2016/04/05 21:24:37]

This validates that we've had 1 call into web_ui (a call to get the default
value) and that the callback back to JS returned true.

+
+  // Set the default to 'Blocked'.
+  base::ListValue setArgs;
+  setArgs.Append(new base::FundamentalValue(type));
+  setArgs.Append(new base::FundamentalValue(CONTENT_SETTING_BLOCK));
+  handler->HandleSetDefaultValueForContentType(&setArgs);
+
+  EXPECT_EQ(2U, web_ui()->call_data().size());

[michaelpg, 2016/04/01 09:08:28]

is this the change event? why not verify the callback here too?

[Finnur, 2016/04/05 10:22:31]

Umm, I'm not sure which callback you are referring to...
HandleSetDefaultValueForContentType does not call back into JavaScript.

[michaelpg, 2016/04/05 19:33:36]

so where does this 2nd call into webui come from? I'm just confused.

[Finnur, 2016/04/05 21:24:37]

This validates that we've had 2 calls from JS into web_ui (first a call to get a
default value and now another, to set the default value). If SetDefault were to
result in a callback call back into to js (which it doesn't) then I would have
created a helper that runs line 118 and then also validates the result of the
callback. But since there is no callback, it suffices to check the number of
calls into web_ui.

To clarify: The term callback, that I use, is referring only to web_ui
communicating back to js with what the result of the web_ui call was.

Another way to think about it: If GetDefault on line 109 did not return a
callback then I'd replace the ValidateDefault function on line 110 with:

EXPECT_EQ(1U, web_ui()->call_data().size());


Does this answer your question? Happy to do a follow-up if you think something
needs clarification/improvement.

[michaelpg, 2016/04/05 23:16:05]

So, you're verifying that Get calls WebUI (via the cr.webUIResponse callback)
and that the callback itself is invoked correctly (the arguments to
ResolveJavascriptCallback which get used in cr.webUIResponse).

You're also verifying that Set results in a WebUI call (from the content
observer), but you aren't verifying the arguments of that call (the
webUIListenerCallback for contentSettingCategoryChanged).

Is my understanding correct? If so, I wasn't misunderstanding what was
happening, just don't know why you didn't test the listener callback when you
did test the response callback. No need to update the test though, it's fine.

+
+  // Verify that the default has been set to 'Blocked'.
+  handler->HandleGetDefaultValueForContentType(&getArgs);
+  ValidateDefault(false, 3U);
+}
+
+TEST_F(SiteSettingsHandlerTest, Origins) {
+  scoped_ptr<SiteSettingsHandler> handler;

[michaelpg, 2016/04/01 09:08:28]

SiteSettingsHandler handler(profile());

[Finnur, 2016/04/05 10:22:31]

Done.

+  handler.reset(new SiteSettingsHandler(profile()));
+  handler->set_web_ui(web_ui());
+
+  ContentSettingsType type = CONTENT_SETTINGS_TYPE_NOTIFICATIONS;
+
+  // Test the JS -> C++ -> JS callback path for configuring origins, by setting
+  // Google.com to blocked.
+  base::ListValue setArgs;
+  std::string google("http://www.google.com");
+  setArgs.Append(new base::StringValue(google));  // Primary pattern.
+  setArgs.Append(new base::StringValue(google));  // Secondary pattern.
+  setArgs.Append(new base::FundamentalValue(type));
+  setArgs.Append(new base::FundamentalValue(CONTENT_SETTING_BLOCK));
+  handler->HandleSetCategoryPermissionForOrigin(&setArgs);
+  EXPECT_EQ(1U, web_ui()->call_data().size());
+
+  // Verify the change was successful.
+  base::ListValue listArgs;
+  listArgs.Append(new base::StringValue("test-callback-id"));
+  listArgs.Append(new base::FundamentalValue(type));
+  handler->HandleGetExceptionList(&listArgs);
+  ValidateOrigin(google, google, "block", "preference", 2U);
+
+  // Reset things back to how they were.
+  base::ListValue resetArgs;
+  resetArgs.Append(new base::StringValue(google));
+  resetArgs.Append(new base::StringValue(google));
+  resetArgs.Append(new base::FundamentalValue(type));
+  handler->HandleResetCategoryPermissionForOrigin(&resetArgs);
+  EXPECT_EQ(3U, web_ui()->call_data().size());
+
+  // Verify the reset was successful.
+  handler->HandleGetExceptionList(&listArgs);
+  ValidateNoOrigin(4U);
+}
+
+}  // namespace settings