screenshot disabling policy tests

chrome/browser/policy/policy_browsertest.cc

Index: chrome/browser/policy/policy_browsertest.cc
diff --git a/chrome/browser/policy/policy_browsertest.cc b/chrome/browser/policy/policy_browsertest.cc
index 80a6f08b6e365caaa7b930dcd3d4fd560794ee61..10d53beca1c0b49222596e6b51896b516f9e5fde 100644
--- a/chrome/browser/policy/policy_browsertest.cc
+++ b/chrome/browser/policy/policy_browsertest.cc
@@ -9,8 +9,8 @@
 #include "base/file_util.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string16.h"
-#include "base/utf_string_conversions.h"
 #include "base/test/test_file_util.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/autocomplete/autocomplete_controller.h"
 #include "chrome/browser/browser_process.h"
@@ -47,6 +47,7 @@
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
@@ -61,6 +62,14 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_CHROMEOS)
+#include "ash/accelerators/accelerator_controller.h"
+#include "ash/accelerators/accelerator_table.h"
+#include "ash/shell.h"
+#include "ash/shell_delegate.h"
+#include "chrome/browser/download/download_prefs.h"
+#endif
+
 using content::BrowserThread;
 using testing::Return;
 
@@ -157,6 +166,54 @@ void DownloadAndVerifyFile(
   EXPECT_EQ(FilePath(), enumerator.Next());
 }
 
+class LoadObserver : public content::NotificationObserver {

[bartfab (slow), 2012/08/31 15:50:13]

#include "content/public/browser/notification_observer.h" is missing for this.

[Joao da Silva, 2012/09/03 09:01:04]

This is essentially a WindowedNotificationObserver (from
content/public/test/test_utils.cc), can't it be used instead?

[qfel, 2012/09/03 11:31:21]

Seems I just missed its existence.

+ public:
+  LoadObserver()
+      : source_(content::NotificationService::AllSources()),
+        message_loop_runner_(new content::MessageLoopRunner) {
+  }
+
+  content::Source<content::NavigationController> Wait() {
+    registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
+                   content::NotificationService::AllSources());
+    message_loop_runner_->Run();
+    return source_;
+  }
+
+  void Observe(int type,
+               const content::NotificationSource& source,
+               const content::NotificationDetails& details) {
+    CHECK_EQ(type, content::NOTIFICATION_LOAD_STOP);

[bartfab (slow), 2012/08/31 15:50:13]

Why CHECK_EQ, not ASSERT_EQ?

[qfel, 2012/09/03 11:31:21]

I wonder myself.. Seems I just happened to see similar code with check when
writing this. Should be assert though.

+    source_ = static_cast<content::Source<content::NavigationController> >(
+        source);
+    message_loop_runner_->Quit();
+  }
+
+ private:
+  content::NotificationRegistrar registrar_;
+  content::Source<content::NavigationController> source_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+};
+
+#if defined(OS_CHROMEOS)
+int CountScreenshots() {
+  DownloadPrefs* download_prefs = DownloadPrefs::FromBrowserContext(

[bartfab (slow), 2012/08/31 15:50:13]

#include "chrome/browser/download/download_prefs.h" is missing for this.

[qfel, 2012/09/03 11:31:21]

It's in the OS_CHROMEOS guarded include section. Moved to unguarded section (now
that I think of it, I recall those kind of included should not be guarded).

+      ash::Shell::GetInstance()->delegate()->GetCurrentBrowserContext());
+  file_util::FileEnumerator enumerator(download_prefs->DownloadPath(),
+                                       false, file_util::FileEnumerator::FILES,
+                                       "Screenshot*");
+  int count = 0;
+  while (!enumerator.Next().empty())
+    count++;
+  return count;
+}
+
+void NoOp() {
+  // As the name suggests, do absolutely nothing and chill out. Used as a
+  // callback to wait for message loop to process previous messages.

[bartfab (slow), 2012/08/31 15:50:13]

This is what base::DoNothing is for.

[qfel, 2012/09/03 11:31:21]

Done.

+}
+#endif
+
 }  // namespace
 
 class PolicyTest : public InProcessBrowserTest {
@@ -175,6 +232,67 @@ class PolicyTest : public InProcessBrowserTest {
         base::Bind(chrome_browser_net::SetUrlRequestMocksEnabled, true));
   }
 
+  void SetScreenshotPolicy(bool enabled) {
+    PolicyMap policies;
+    policies.Set(key::kDisableScreenshots, POLICY_LEVEL_MANDATORY,
+                 POLICY_SCOPE_USER, base::Value::CreateBooleanValue(!enabled));

[bartfab (slow), 2012/08/31 15:50:13]

#include "base/values.h" is missing for this.

[qfel, 2012/09/03 11:31:21]

Done.

+    provider_.UpdateChromePolicy(policies);
+  }
+
+  void TestScreenshotFeedback(bool enabled) {
+    SetScreenshotPolicy(enabled);
+
+    // Wait for feedback page to load.
+    LoadObserver observer;
+    EXPECT_TRUE(chrome::ExecuteCommand(browser(), IDC_FEEDBACK));
+    content::WebContents* web_contents = observer.Wait()->GetWebContents();
+
+    // Wait for feedback page to fully initialize.
+    bool result = false;
+    EXPECT_TRUE(content::ExecuteJavaScriptAndExtractBool(
+        web_contents->GetRenderViewHost(),
+        std::wstring(),

[bartfab (slow), 2012/08/31 15:50:13]

L"" would work just as well.

[qfel, 2012/09/03 11:31:21]

The code where I found ExecuteJavaScriptAndExtractBool used explicit constructor
call, so I decided to do the same.

+        L"function btest_initCompleted(url) {"

[bartfab (slow), 2012/08/31 15:50:13]

Indentation throughout this JS code should be two spaces, not one space.

[qfel, 2012/09/03 11:31:21]

Done.

+        L" var img = new Image();"
+        L" img.src = url;"
+        L" img.onload = function() {"
+        L"  domAutomationController.send(img.width * img.height > 0);"
+        L" };"
+        L" img.onerror = function() {"
+        L"  domAutomationController.send(false);"
+        L" };"
+        L"}"
+        L"function setupCurrentScreenshot(url) {"
+        L" btest_initCompleted(url);"
+        L"}"
+        L"var img = document.getElementById("
+        L"    'current-screenshots-thumbnailDiv-0-image');"
+        L"if (img)"
+        L" btest_initCompleted(img.src);",

[Joao da Silva, 2012/09/03 09:01:04]

else
  domAutomationController.send(false);

[qfel, 2012/09/03 11:31:21]

No. If !img, then setupCurrentScreenshot is yet to be called, so do nothing.

[Joao da Silva, 2012/09/03 11:37:55]

Right. The comment makes that clear now :-)

+        &result));
+    EXPECT_TRUE(result == enabled);

[Joao da Silva, 2012/09/03 09:01:04]

EXPECT_EQ

[qfel, 2012/09/03 11:31:21]

Done.

+
+    // Feedback page is a singleton page, so close so future calls to this
+    // function work as expected.
+    web_contents->Close();
+  }
+
+#if defined(OS_CHROMEOS)
+  void TestScreenshotFile(bool enabled) {
+    SetScreenshotPolicy(enabled);
+    ash::Shell::GetInstance()->accelerator_controller()->PerformAction(
+      ash::TAKE_SCREENSHOT, ui::Accelerator());

[Joao da Silva, 2012/09/03 09:01:04]

indent

[qfel, 2012/09/03 11:31:21]

Done.

+
+    // TAKE_SCREENSHOT handler posts write file task on success, wait for it.
+    BrowserThread::PostTaskAndReply(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(NoOp),
+        MessageLoop::QuitClosure());
+    content::RunMessageLoop();
+  }
+#endif
+
   MockConfigurationPolicyProvider provider_;
 };
 
@@ -485,4 +603,27 @@ IN_PROC_BROWSER_TEST_F(PolicyTest, URLBlacklist) {
   CheckCanOpenURL(browser(), kBBB_PATH);
 }
 
+IN_PROC_BROWSER_TEST_F(PolicyTest, DisableScreenshotsFeedback) {
+  // Make sure current screenshot can be taken and displayed on feedback page.
+  TestScreenshotFeedback(true);
+
+  // Check if banning screenshots disabled feedback page's ability to grab a
+  // screenshot.
+  TestScreenshotFeedback(false);
+}
+
+#if defined(OS_CHROMEOS)
+IN_PROC_BROWSER_TEST_F(PolicyTest, DisableScreenshotsFile) {
+  int screenshot_count = CountScreenshots();
+
+  // Make sure screenshots are counted correctly.
+  TestScreenshotFile(true);
+  ASSERT_EQ(CountScreenshots(), screenshot_count + 1);
+
+  // Check if trying to take a screenshot fails when disabled by policy.
+  TestScreenshotFile(false);
+  ASSERT_EQ(CountScreenshots(), screenshot_count + 1);
+}
+#endif
+
 }  // namespace policy