Improve <adview> implementation and add tests.

chrome/browser/extensions/ad_view_browsertest.cc

Index: chrome/browser/extensions/ad_view_browsertest.cc
diff --git a/chrome/browser/extensions/ad_view_browsertest.cc b/chrome/browser/extensions/ad_view_browsertest.cc
index 4a70524d7600e530925692b884fdcae56a545410..909a513d7c8fd3875b1a92050db4237cc5539b4d 100644
--- a/chrome/browser/extensions/ad_view_browsertest.cc
+++ b/chrome/browser/extensions/ad_view_browsertest.cc
@@ -5,6 +5,9 @@
 #include "chrome/browser/extensions/extension_test_message_listener.h"
 #include "chrome/browser/extensions/platform_app_browsertest_util.h"
 #include "chrome/common/chrome_switches.h"
+#include "content/test/net/url_request_prepackaged_interceptor.h"
+#include "net/url_request/url_fetcher.h"
+
 
 class AdViewTest : public extensions::PlatformAppBrowserTest {
  protected:
@@ -15,28 +18,113 @@ class AdViewTest : public extensions::PlatformAppBrowserTest {
   }
 };
 
-IN_PROC_BROWSER_TEST_F(AdViewTest, LoadEventIsCalled) {
+IN_PROC_BROWSER_TEST_F(AdViewTest, LoadCommitEventIsCalled) {

[benwells, 2013/04/02 00:49:45]

Where did all the comments go?

[rpaquay, 2013/04/02 16:44:47]

I moved them in their respective "chrometest.js" file, because the remaining
test code in this file is now pretty mechanical and uninteresting.

[benwells, 2013/04/03 02:40:03]

This is the first point of investigation for anyone looking into test failures.
Comments here are very useful if it is not obvious what a test is testing.

I think you should bring the comments back.

[rpaquay, 2013/04/03 18:30:05]

Done.

+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/loadcommit_event")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(AdViewTest, LoadAbortEventIsCalled) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/loadabort_event")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(AdViewTest, CommitMessageFromAdNetwork) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/onloadcommit_ack")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(AdViewTest, DisplayFirstAd) {
   ASSERT_TRUE(StartTestServer());
 
-  ExtensionTestMessageListener listener("guest-loaded", false);
-  LoadAndLaunchPlatformApp("ad_view/load_event");
-  ASSERT_TRUE(listener.WaitUntilSatisfied());
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/display_first_ad")) << message_;
 }
 
-IN_PROC_BROWSER_TEST_F(AdViewTest, AdNetworkIsLoaded) {
+IN_PROC_BROWSER_TEST_F(AdViewTest, PropertiesAreInSyncWithAttributes) {
   ASSERT_TRUE(StartTestServer());
 
-  ExtensionTestMessageListener listener("ad-network-loaded", false);
-  LoadAndLaunchPlatformApp("ad_view/ad_network_loaded");
-  ASSERT_TRUE(listener.WaitUntilSatisfied());
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/properties_exposed")) << message_;
 }
 
-// This test currently fails on trybots because it requires new binary file
-// (image315.png). The test will be enabled once the binary files are committed.
-IN_PROC_BROWSER_TEST_F(AdViewTest, DISABLED_DisplayFirstAd) {
+IN_PROC_BROWSER_TEST_F(AdViewTest, AdViewPermissionIsRequired) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/permission_required")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(AdViewTest, ChangeAdNetworkValue) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/change_ad_network")) << message_;
+}
+
+
+class AdViewNoSrcTest : public extensions::PlatformAppBrowserTest {
+ protected:
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    extensions::PlatformAppBrowserTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(switches::kEnableAdview);
+    //Note: The "kEnableAdviewSrcAttribute" flag is not here!
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(AdViewNoSrcTest, InvalidAdNetworkIsIgnored) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/invalid_ad_network")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(AdViewNoSrcTest, EnableAdviewSrcAttributeFlagRequired) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/src_flag_required")) << message_;
+}
+
+// This test checks an <adview> works end-to-end (i.e. page is loaded) when
+// using a whitelisted ad-network.
+IN_PROC_BROWSER_TEST_F(AdViewNoSrcTest, SrcNotExposed) {
+  base::FilePath file_path = test_data_dir_
+    .AppendASCII("platform_apps")
+    .AppendASCII("ad_view/src_not_exposed")
+    .AppendASCII("ad_network_fake_website.html");
+
+  // Note: The following URL is identical to the whitelisted url
+  //       for "admob" (see ad_view.js).
+  GURL url = GURL("https://admob-sdk.doubleclick.net/chromeapps");
+  std::string scheme = url.scheme();
+  std::string hostname = url.host();
+
+  content::URLRequestPrepackagedInterceptor interceptor(scheme, hostname);
+  interceptor.SetResponse(url, file_path);
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/src_not_exposed")) << message_;
+  ASSERT_EQ(1, interceptor.GetHitCount());
+}
+
+
+class AdViewNotEnabledTest : public extensions::PlatformAppBrowserTest {
+ protected:
+  virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
+    extensions::PlatformAppBrowserTest::SetUpCommandLine(command_line);
+    //Note: The "kEnableAdview" flag is not here!
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(AdViewNotEnabledTest, EnableAdviewFlagRequired) {
   ASSERT_TRUE(StartTestServer());
 
-  ExtensionTestMessageListener listener("ad-displayed", false);
-  LoadAndLaunchPlatformApp("ad_view/display_first_ad");
-  ASSERT_TRUE(listener.WaitUntilSatisfied());
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/flag_required")) << message_;
 }