Improve <adview> implementation and add tests.

chrome/test/data/extensions/platform_apps/ad_view/invalid_ad_network/chrometest.js

Index: chrome/test/data/extensions/platform_apps/ad_view/invalid_ad_network/chrometest.js
diff --git a/chrome/test/data/extensions/platform_apps/ad_view/invalid_ad_network/chrometest.js b/chrome/test/data/extensions/platform_apps/ad_view/invalid_ad_network/chrometest.js
new file mode 100644
index 0000000000000000000000000000000000000000..c4c6a78fa4ec5f700f851414af98eb31d6c77e6c
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/ad_view/invalid_ad_network/chrometest.js
@@ -0,0 +1,77 @@
+// Copyright (c) 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.
+
+// This test checks an invalid "ad-network" value (i.e. not whitelisted)
+// is ignored.
+
+var adview1 = "adview1";
+var adview2 = "adview2";
+var adview3 = "adview3";
+
+/**
+ * Verify setting invalid ad-network value is ignored in immediate Javascript
+ * code (i.e. before document is fully loaded) on a static <adview> element.
+ */
+function runTest1() {
+  var adview = document.getElementById(adview1);
+
+  adview.setAttribute("ad-network", "invalidAdNetwork");
+
+  // Note: The assertion has to happen in a callback function because of the
+  //  way mutation observer events are processed. See
+  //  http://lists.w3.org/Archives/Public/public-webapps/2011JulSep/1622.html
+  setTimeout(function() {
+    chrome.test.assertEq("", adview.getAttribute("ad-network"));
+    console.log("test1 succeeded.");
+  }, 0);
+}
+
+/**
+ * Verify setting invalid ad-network value is ignored at "document load" time.
+ */
+function runTest2() {
+  var adview = document.createElement("adview");
+  adview.id = adview2;
+  document.body.appendChild(adview);
+
+  adview.setAttribute("ad-network", "invalidAdNetwork2");
+
+  // Note: The assertion has to happen in a callback function because of the
+  //  way mutation observer events are processed. See
+  //  http://lists.w3.org/Archives/Public/public-webapps/2011JulSep/1622.html
+  setTimeout(function() {
+    chrome.test.assertEq("", adview.getAttribute("ad-network"));
+    console.log("test2 succeeded.");
+  }, 0);
+}
+
+/**
+ * Verify setting invalid ad-network value is ignored even after document
+ * is fully loaded.
+ */
+function runTest3() {
+  setTimeout(function() {

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

I think you are avoiding chrome.runTests so can ensure test 1 happens before
document load and the others after.

Instead of putting in this timeout, you should call runTest3 when test 2
finishes. That would be clearer and more robust.

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

In this case, I was using "setTimeout" in "runTest3" for a different purpose: I
wanted to run code *after* the "DOMContentLoaded" event if fired and processed
by all listeners -- the reason being that ad_view.js contains code watching for
DOM change events which depends on when "DOMContentLoaded" is fired. Is there a
better way to achieve what I am trying to test?

[benwells, 2013/04/04 04:33:52]

OK, I see. Thanks for adding the comment, they really help with tests like this
where people unfamiliar with this area maybe end up working out why the test is
failing.

This setup feels slightly fragile though. It is currently the case that test 2
and test 3 will be running simultaneously. I don't think it can happen now, but
with some slight changes (e.g. one level of callbacks added) test 2 might finish
after test 3 has finished. This could mean a failure in test 2 would not get
caught.

It would be better to start test 3 once test 2 has completed. That would
guarantee the tests run sequentially, and also would ensure that test 3 happens
after the loaded event has been processed (but you should keep an updated
comment explaining so if test 2 is ever removed a timeout is added back).

[rpaquay, 2013/04/05 16:12:08]

I see what you mean. I made a change that should enforces test3 runs only after
test2 succeeds.

+    var adview = document.createElement("adview");
+    adview.id = adview3;
+    document.body.appendChild(adview);
+
+    adview.setAttribute("ad-network", "invalidAdNetwork3");
+
+    // Note: The assertion has to happen in a callback function because of the
+    //  way mutation observer events are processed. See
+    //  http://lists.w3.org/Archives/Public/public-webapps/2011JulSep/1622.html
+    setTimeout(function() {
+      chrome.test.assertEq("", adview.getAttribute("ad-network"));
+      console.log("test3 succeeded.");
+      chrome.test.succeed();
+    }, 0);
+  }, 0);
+}
+
+runTest1();
+
+window.addEventListener('DOMContentLoaded', function() {
+  runTest2();
+  runTest3();
+});