Add browsertest for Flash permission prompt

chrome/browser/permissions/permissions_browsertest.cc

Index: chrome/browser/permissions/permissions_browsertest.cc
diff --git a/chrome/browser/permissions/permissions_browsertest.cc b/chrome/browser/permissions/permissions_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..2149572f7dd1ccd4d0b3727914405a330fb0d969
--- /dev/null
+++ b/chrome/browser/permissions/permissions_browsertest.cc
@@ -0,0 +1,182 @@
+// 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/command_line.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/engagement/site_engagement_score.h"
+#include "chrome/browser/permissions/permission_request_manager.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/website_settings/mock_permission_prompt_factory.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/common/content_switches.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/ppapi_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "ppapi/shared_impl/ppapi_switches.h"
+
+// This is a class which defines how an individual feature that has a permission
+// should be tested. Basically there are two functions that should be overriden
+// for each feature: (1) TriggerPrompt which should do whatever is needed to
+// cause a permission prompt for the feature; and (2) FeatureUsageSucceeds which
+// should return true if the feature works (i.e. has permission). Typically
+// these functions would call into javascript on the current page to manipulate
+// the page in a way which tests these things.
+class PermissionTestConfig {

[tommycli, 2016/09/26 22:54:49]

The abstraction seems good - and I don't know permissions code that well - but
are we definitely going to reuse this interface?

Because if not, maybe we should follow the You Ain't Gonna Need It and just have
one concrete test for Flash. It could be a lot simpler

Or - if we are going to reuse it, it might be nice to put the test config and
PermissionsBrowserTest in a test support file. That can be in a separate patch
if that makes it easier...

The test just seems complex.

[raymes, 2016/09/27 04:05:41]

Yes we'll definitely reuse it - I have an outstanding bug to add a framework
like this: see https://bugs.chromium.org/p/chromium/issues/detail?id=644052

I can move FlashPermissionTestConfig into a separate file if you like? 

+ public:
+  virtual void TriggerPrompt() = 0;
+  virtual bool FeatureUsageSucceeds() = 0;
+
+  std::string test_name() const { return test_name_; }
+  std::string test_url() const { return test_url_; }
+
+  void InitBrowser(Browser* browser) {
+    DCHECK(!browser_);
+    browser_ = browser;
+  }
+
+  virtual void SetUpCommandLine(base::CommandLine* command_line) {}
+  virtual void SetUpOnMainThread() {}
+
+ protected:
+  explicit PermissionTestConfig(const std::string& test_name,
+                                const std::string& test_url)
+      : test_name_(test_name), test_url_(test_url), browser_(nullptr) {}
+
+  bool RunScriptReturnBool(const std::string& script) {
+    bool result;
+    EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
+        GetWebContents()->GetMainFrame(), script, &result));
+    return result;
+  }
+
+  Browser* browser() { return browser_; }
+
+  content::WebContents* GetWebContents() {
+    return browser()->tab_strip_model()->GetActiveWebContents();
+  }
+
+ private:
+  std::string test_name_;
+  std::string test_url_;
+  Browser* browser_;
+};
+
+class FlashPermissionTestConfig : public PermissionTestConfig {
+ public:
+  FlashPermissionTestConfig()
+      : PermissionTestConfig("Flash", "permissions/flash.html") {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(switches::kEnablePepperTesting);

[tommycli, 2016/09/26 22:54:49]

Would recommend inlining kEnablePepperTesting within RegisterFlashTestPlugin,
since that plugin crashes without that flag

[raymes, 2016/09/27 04:05:41]

Done.

+    command_line->AppendSwitch(switches::kEnablePluginPlaceholderTesting);
+    command_line->AppendSwitchASCII(
+        switches::kOverridePluginPowerSaverForTesting, "ignore-list");

[tommycli, 2016/09/26 22:54:49]

ignore-list unneeded if we're registering the plugin as Flash anyways.

[raymes, 2016/09/27 04:05:41]

Done.

+
+    ASSERT_TRUE(ppapi::RegisterFlashTestPlugin(command_line));
+
+    feature_list_.InitAndEnableFeature(features::kPreferHtmlOverPlugins);
+  }
+
+  void SetUpOnMainThread() override {
+    SiteEngagementScore::SetParamValuesForTesting();
+  }
+
+  void TriggerPrompt() override {
+    EXPECT_TRUE(RunScriptReturnBool("triggerPrompt();"));
+  }
+
+  bool FeatureUsageSucceeds() override {
+    // Flash won't be enabled until the page is refreshed.
+    ui_test_utils::NavigateToURL(browser(),
+                                 GetWebContents()->GetLastCommittedURL());
+    return RunScriptReturnBool("flashIsEnabled();");
+  }
+
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// This is a generic test for testing features that have permissions. It will
+// test cases such as what will happen if a permission prompt is
+// allowed/denied/dismissed for a feature.
+class PermissionsBrowserTest : public InProcessBrowserTest {
+ public:
+  // TODO(raymes): paramaterize this to work with multiple configs.
+  PermissionsBrowserTest() : current_test_(new FlashPermissionTestConfig) {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    current_test_->SetUpCommandLine(command_line);
+  }
+
+  void SetUpInProcessBrowserTestFixture() override {}

[dominickn, 2016/09/26 07:28:59]

Nit: remove the do-nothing override?

[raymes, 2016/09/27 04:05:41]

Done.

+
+  void SetUpOnMainThread() override {
+    InProcessBrowserTest::SetUpOnMainThread();
+
+    PermissionRequestManager* manager =
+        PermissionRequestManager::FromWebContents(
+            browser()->tab_strip_model()->GetActiveWebContents());
+    prompt_factory_.reset(new MockPermissionPromptFactory(manager));
+    manager->DisplayPendingRequests();
+
+    current_test_->InitBrowser(browser());
+    current_test_->SetUpOnMainThread();
+
+    ASSERT_TRUE(embedded_test_server()->Start());
+
+    ui_test_utils::NavigateToURL(
+        browser(), embedded_test_server()->GetURL(current_test_->test_url()));
+
+    LOG(INFO) << "Running test for: " << current_test_->test_name();

[dominickn, 2016/09/26 07:28:59]

Nit: remove LOG

[raymes, 2016/09/27 04:05:41]

The log will be important when there is a paramaterized test because all you
will see in the test output is a number denoting which test is running. This
will give more context.

+  }
+
+  void TearDownOnMainThread() override {
+    prompt_factory_.reset();
+
+    InProcessBrowserTest::TearDownOnMainThread();
+  }
+
+  MockPermissionPromptFactory* prompt_factory() {
+    return prompt_factory_.get();
+  }
+
+  PermissionTestConfig* current_test() { return current_test_.get(); }
+
+ private:
+  std::unique_ptr<MockPermissionPromptFactory> prompt_factory_;
+  std::unique_ptr<PermissionTestConfig> current_test_;
+};
+
+IN_PROC_BROWSER_TEST_F(PermissionsBrowserTest, FailsBeforeRequesting) {
+  // Dismiss any prompts if they are shown when using the feature.
+  prompt_factory()->set_response_type(PermissionRequestManager::DISMISS);

[dominickn, 2016/09/26 07:28:59]

Nit: check that the current count is 0?

[raymes, 2016/09/27 04:05:41]

Done.

+  EXPECT_FALSE(current_test()->FeatureUsageSucceeds());
+}
+
+IN_PROC_BROWSER_TEST_F(PermissionsBrowserTest, FailsIfDismissed) {
+  prompt_factory()->set_response_type(PermissionRequestManager::DISMISS);
+  current_test()->TriggerPrompt();
+
+  EXPECT_EQ(1, prompt_factory()->total_request_count());
+  EXPECT_FALSE(current_test()->FeatureUsageSucceeds());
+}
+
+IN_PROC_BROWSER_TEST_F(PermissionsBrowserTest, FailsIfBlocked) {
+  prompt_factory()->set_response_type(PermissionRequestManager::DENY_ALL);
+  current_test()->TriggerPrompt();
+
+  EXPECT_EQ(1, prompt_factory()->total_request_count());
+  EXPECT_FALSE(current_test()->FeatureUsageSucceeds());
+}
+
+IN_PROC_BROWSER_TEST_F(PermissionsBrowserTest, SucceedsIfAllowed) {
+  prompt_factory()->set_response_type(PermissionRequestManager::ACCEPT_ALL);
+  current_test()->TriggerPrompt();
+
+  EXPECT_EQ(1, prompt_factory()->total_request_count());
+  EXPECT_TRUE(current_test()->FeatureUsageSucceeds());
+}