Improve <adview> implementation and add tests.

The first CL for bug 180618 contained initial support for the <adview> tag. This is a follow up CL to

* fix issues related to various combinations of flags, permissions and property/attributes behavior,
* add corresponding tests,
* and improve code readability of "ad_view.js"

BUG=226125
TBR=asargent@chromium.org (for trivial changes in chrome/browser/component_updater/test/component_updater_service_unittest.cc)

Review URL: https://chromiumcodereview.appspot.com/12967016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@196036 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 139 insertions, 28 deletions

diff --git a/chrome/browser/component_updater/test/component_updater_service_unittest.cc b/chrome/browser/component_updater/test/component_updater_service_unittest.cc
index 6689e49..ce85ad1 100644
--- a/chrome/browser/component_updater/test/component_updater_service_unittest.cc
+++ b/chrome/browser/component_updater/test/component_updater_service_unittest.cc
@@ -294,7 +294,7 @@ TEST_F(ComponentUpdaterTest, CheckCrxSleep) {
   io_thread.StartIOThread();
   file_thread.Start();
 
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
 
   CrxComponent com;
   EXPECT_EQ(ComponentUpdateService::kOk,
@@ -373,7 +373,7 @@ TEST_F(ComponentUpdaterTest, InstallCrx) {
   io_thread.StartIOThread();
   file_thread.Start();
 
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
 
   CrxComponent com1;
   RegisterComponent(&com1, kTestComponent_jebg, Version("0.9"));
@@ -438,7 +438,7 @@ TEST_F(ComponentUpdaterTest, InstallCrxTwoSources) {
   io_thread.StartIOThread();
   file_thread.Start();
 
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
 
   CrxComponent com1;
   RegisterComponent(&com1, kTestComponent_abag, Version("2.2"));
@@ -510,7 +510,7 @@ TEST_F(ComponentUpdaterTest, ProdVersionCheck) {
   io_thread.StartIOThread();
   file_thread.Start();
 
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
 
   CrxComponent com;
   RegisterComponent(&com, kTestComponent_jebg, Version("0.9"));
@@ -550,7 +550,7 @@ TEST_F(ComponentUpdaterTest, CheckForUpdateSoon) {
   io_thread.StartIOThread();
   file_thread.Start();
 
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
 
   CrxComponent com1;
   RegisterComponent(&com1, kTestComponent_abag, Version("2.2"));
@@ -666,7 +666,7 @@ TEST_F(ComponentUpdaterTest, CheckReRegistration) {
   io_thread.StartIOThread();
   file_thread.Start();
 
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
 
   CrxComponent com1;
   RegisterComponent(&com1, kTestComponent_jebg, Version("0.9"));
diff --git a/chrome/browser/extensions/ad_view_browsertest.cc b/chrome/browser/extensions/ad_view_browsertest.cc
index bb6310a..0391268 100644
--- a/chrome/browser/extensions/ad_view_browsertest.cc
+++ b/chrome/browser/extensions/ad_view_browsertest.cc
@@ -1,10 +1,12 @@
-// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Copyright 2013 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/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,34 +17,143 @@ class AdViewTest : public extensions::PlatformAppBrowserTest {
   }
 };
 
-IN_PROC_BROWSER_TEST_F(AdViewTest, LoadEventIsCalled) {
+// This test checks the "loadcommit" event is called when the page inside an
+// <adview> is loaded.
+IN_PROC_BROWSER_TEST_F(AdViewTest, LoadCommitEventIsCalled) {
   ASSERT_TRUE(StartTestServer());
 
-  ExtensionTestMessageListener listener("guest-loaded", false);
-  LoadAndLaunchPlatformApp("ad_view/load_event");
-  ASSERT_TRUE(listener.WaitUntilSatisfied());
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/loadcommit_event")) << message_;
 }
 
+// This test checks the "loadabort" event is called when the "src" attribute
+// of an <adview> is an invalid URL.
+IN_PROC_BROWSER_TEST_F(AdViewTest, LoadAbortEventIsCalled) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/loadabort_event")) << message_;
+}
+
+// This test checks the page loaded inside an <adview> has the ability to
+// 1) receive "message" events from the application, and 2) use
+// "window.postMessage" to post back a message to the application.
 #if defined(OS_WIN)
 // Flaky, or takes too long time on Win7. (http://crbug.com/230271)
-#define MAYBE_AdNetworkIsLoaded DISABLED_AdNetworkIsLoaded
+#define MAYBE_CommitMessageFromAdNetwork DISABLED_CommitMessageFromAdNetwork
 #else
-#define MAYBE_AdNetworkIsLoaded AdNetworkIsLoaded
+#define MAYBE_CommitMessageFromAdNetwork CommitMessageFromAdNetwork
 #endif
-IN_PROC_BROWSER_TEST_F(AdViewTest, MAYBE_AdNetworkIsLoaded) {
+IN_PROC_BROWSER_TEST_F(AdViewTest, MAYBE_CommitMessageFromAdNetwork) {
   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/onloadcommit_ack")) << 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.
+// This test checks the page running inside an <adview> has the ability to load
+// and display an image inside an <iframe>.
+// Note: Disabled for initial checkin because the test depends on a binary
+//       file (image035.png) which the trybots don't process correctly when
+//       first checked-in.
 IN_PROC_BROWSER_TEST_F(AdViewTest, DISABLED_DisplayFirstAd) {
   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/display_first_ad")) << message_;
+}
+
+// This test checks that <adview> attributes are also exposed as properties
+// (with the same name and value).
+IN_PROC_BROWSER_TEST_F(AdViewTest, PropertiesAreInSyncWithAttributes) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/properties_exposed")) << message_;
+}
+
+// This test checks an <adview> element has no behavior when the "adview"
+// permission is missing from the application manifest.
+IN_PROC_BROWSER_TEST_F(AdViewTest, AdViewPermissionIsRequired) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/permission_required")) << message_;
+}
+
+// This test checks that 1) it is possible change the value of the "ad-network"
+// attribute of an <adview> element and 2) changing the value will reset the
+// "src" attribute.
+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!
+  }
+};
+
+// This test checks an invalid "ad-network" value (i.e. not whitelisted)
+// is ignored.
+IN_PROC_BROWSER_TEST_F(AdViewNoSrcTest, InvalidAdNetworkIsIgnored) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/invalid_ad_network")) << message_;
+}
+
+// This test checks the "src" attribute is ignored when the
+// "kEnableAdviewSrcAttribute" is missing.
+IN_PROC_BROWSER_TEST_F(AdViewNoSrcTest, EnableAdviewSrcAttributeFlagRequired) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/src_flag_required")) << message_;
+}
+
+// This test checks 1) an <adview> works end-to-end (i.e. page is loaded) when
+// using a whitelisted ad-network, and 2) the "src" attribute is never exposed
+// to the application.
+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!
+  }
+};
+
+// This test checks an <adview> element has no behavior when the "kEnableAdview"
+// flag is missing.
+IN_PROC_BROWSER_TEST_F(AdViewNotEnabledTest, EnableAdviewFlagRequired) {
+  ASSERT_TRUE(StartTestServer());
+
+  ASSERT_TRUE(RunPlatformAppTest(
+      "platform_apps/ad_view/flag_required")) << message_;
 }
diff --git a/chrome/browser/extensions/api/management/management_browsertest.cc b/chrome/browser/extensions/api/management/management_browsertest.cc
index b2dc049..03b0514 100644
--- a/chrome/browser/extensions/api/management/management_browsertest.cc
+++ b/chrome/browser/extensions/api/management/management_browsertest.cc
@@ -250,7 +250,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, MAYBE_AutoUpdate) {
   NotificationListener notification_listener;
   base::FilePath basedir = test_data_dir_.AppendASCII("autoupdate");
   // Note: This interceptor gets requests on the IO thread.
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
   net::URLFetcher::SetEnableInterceptionForTests(true);
 
   interceptor.SetResponseIgnoreQuery(
@@ -336,7 +336,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest,
   NotificationListener notification_listener;
   base::FilePath basedir = test_data_dir_.AppendASCII("autoupdate");
   // Note: This interceptor gets requests on the IO thread.
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
   net::URLFetcher::SetEnableInterceptionForTests(true);
 
   interceptor.SetResponseIgnoreQuery(
@@ -412,7 +412,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, MAYBE_ExternalUrlUpdate) {
   base::FilePath basedir = test_data_dir_.AppendASCII("autoupdate");
 
   // Note: This interceptor gets requests on the IO thread.
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
   net::URLFetcher::SetEnableInterceptionForTests(true);
 
   interceptor.SetResponseIgnoreQuery(
@@ -498,7 +498,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, ExternalPolicyRefresh) {
   base::FilePath basedir = test_data_dir_.AppendASCII("autoupdate");
 
   // Note: This interceptor gets requests on the IO thread.
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
   net::URLFetcher::SetEnableInterceptionForTests(true);
 
   interceptor.SetResponseIgnoreQuery(
@@ -573,7 +573,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest,
   ASSERT_TRUE(service->disabled_extensions()->is_empty());
 
   // Note: This interceptor gets requests on the IO thread.
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
   net::URLFetcher::SetEnableInterceptionForTests(true);
 
   interceptor.SetResponseIgnoreQuery(
diff --git a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
index 215ef7d..7c8934a 100644
--- a/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
+++ b/chrome/browser/extensions/extension_disabled_ui_browsertest.cc
@@ -189,7 +189,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
   InstallIncreasingPermissionExtensionV1();
 
   // Note: This interceptor gets requests on the IO thread.
-  content::URLRequestPrepackagedInterceptor interceptor;
+  content::URLLocalHostRequestPrepackagedInterceptor interceptor;
   net::URLFetcher::SetEnableInterceptionForTests(true);
   interceptor.SetResponseIgnoreQuery(
       GURL("http://localhost/autoupdate/updates.xml"),