Add browser_test for extension app API with missing schema

chrome/test/data/extensions/api_test/stubs_app/background.js

Index: chrome/test/data/extensions/api_test/stubs_app/background.js
diff --git a/chrome/test/data/extensions/api_test/stubs_app/background.js b/chrome/test/data/extensions/api_test/stubs_app/background.js
new file mode 100644
index 0000000000000000000000000000000000000000..4b55123f7696af94ddb1aaa79c82ae9190850ab4
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/stubs_app/background.js
@@ -0,0 +1,110 @@
+// Copyright 2014 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.
+
+var apiFeatures = chrome.test.getApiFeatures();
+
+// Returns true if the feature is available to platform apps, either via a
+// specific path or via an entire namespace.
+function isAvailableToPlatformApps(namespace, path) {
+  function checkFeature(feature) {
+    return typeof(feature.extension_types) == "undefined" ||

[not at google - send to devlin, 2014/05/02 22:19:57]

this first check isn't right, since if it's undefined you need to step down into
the dependencies, and then you're in a world you don't want to be. most APIs'
availability is defined by a dependency on a permission (often a manifest key).

so this test has rotted a bit. I'm surprised this even passes, since this line
means that basically every API will get added anyway, and why does that not
cause line 72 to fail?

in other words you may as well remove this isAvailabileToPlatformApps check
entirely?

more robust would be to expose a function which returns all APIs that an
extension has access to, from the test API. all of the dependency logic is
implemented. I think that would be a good way to do it?

[James Cook, 2014/05/02 23:59:15]

I took out this isAvailableToPlatformApps check and just iterate over all APIs.

+        feature.extension_types.indexOf('all') != -1 ||

[not at google - send to devlin, 2014/05/02 22:19:57]

however, this test could be much stronger if the sample app actually requested
permissions. without requesting permissions it basically should have access to
chrome.app.runtime and chrome.app.window. that's not very interesting.

you should be able to get a decent approximation by looping through all
permissions and adding those to the "permissions" field of the manifest.

or, as a shortcut for now (... and likely all time), just copying the app REPL
permissions/manifest.

[James Cook, 2014/05/02 23:59:15]

Added permissions from app REPL and a pass through _permission_features.json
that picked up a few more. This test now checks a lot more APIs. :-)

[not at google - send to devlin, 2014/05/03 00:12:43]

Awesome :)

though won't pick up on people accidentally not adding schemas for new APIs.
hopefully they will test for it. it's the removing of APIs that has broken us
historically (... those 2 times it has happened).

+        feature.extension_types.indexOf('platform_app') != -1;
+  }
+
+  if (apiFeatures.hasOwnProperty(path))
+    return checkFeature(apiFeatures[path]);
+  return apiFeatures.hasOwnProperty(namespace) &&
+      checkFeature(apiFeatures[namespace]);
+}
+
+// Returns a list of all chrome.foo.bar API paths available to an app.
+function getApiPaths() {
+  var apiPaths = [];
+  var apiDefinitions = chrome.test.getApiDefinitions();
+  apiDefinitions.forEach(function(module) {
+    var namespace = module.namespace;
+
+    // Get the API functions and events.
+    ["functions", "events"].forEach(function(section) {
+      if (typeof(module[section]) == "undefined")
+        return;
+      module[section].forEach(function(entry) {
+        var path = namespace + "." + entry.name;
+        if (isAvailableToPlatformApps(namespace, path)) {
+          apiPaths.push(path);
+        }
+      });
+    });
+
+    // Get the API properties.
+    if (module.properties) {
+      for (var propName in module.properties) {

[not at google - send to devlin, 2014/05/02 22:19:57]

Object.getOwnPropertyNames(module.properties).forEach

is safer, doesn't step up proto chain.

[James Cook, 2014/05/02 23:59:15]

Done.

+        var path = namespace + "." + propName;
+        if (isAvailableToPlatformApps(namespace, path)) {
+          apiPaths.push(path);
+        }
+      }
+    }
+  });
+  return apiPaths;
+}
+
+// Tests whether all parts of an API path can be accessed. The path is a
+// namespace or function/property/event etc. within a namespace, and is
+// dot-separated.
+function testPath(path) {
+  var parts = path.split('.');
+
+  var module = chrome;
+  for (var i = 0; i < parts.length; i++) {
+    // Touch this component of the path. This will die if an API does not have
+    // a schema registered.
+    module = module[parts[i]];
+
+    if (i < parts.length - 1) {
+      // Not the last component. Allowed to be undefined because some paths are

[not at google - send to devlin, 2014/05/02 22:19:57]

this check confuses me.

[James Cook, 2014/05/02 23:01:39]

A quick question while I'm working on the rest of it...

This check confuses me as well. I copied it over from the other api_tests/stubs
test. It seems to be needed when testing the path of APIs like:

app.currentWindowInternal.focus

When iterating over the path it finds "currentWindowInternal" is undefined.  Is
that because I'm screwing up how I get the list of possible paths?  Do I need to
filter out internal APIs?

[not at google - send to devlin, 2014/05/02 23:03:17]

ah yeah you need to filter out internal APIs.

[James Cook, 2014/05/02 23:59:15]

Filtering out internal APIs gets rid of the need for this insanity. Done.

+      // only defined on some platforms.
+      if (typeof(module) == "undefined")
+        return true;
+    } else {
+      // This is the last component. It should be defined, unless it is the
+      // lastError property, which depends on there being an error.
+      if (typeof(module) == "undefined" &&
+          path != "extension.lastError" &&
+          path != "runtime.lastError") {
+        console.log(" fail (should not be undefined): " + path);
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+function doTest() {
+  var apiPaths = getApiPaths();

[not at google - send to devlin, 2014/05/02 22:19:57]

I would just inline this

[James Cook, 2014/05/02 23:59:15]

Done.

+  var failures = [];
+  var success = true;
+
+  // Run over each string path in apiPaths and ensure each path is defined.
+  apiPaths.forEach(function(path) {
+    if (!testPath(path)) {
+      success = false;
+      failures.push(path);
+    }
+  });
+
+  if (success) {

[not at google - send to devlin, 2014/05/02 22:19:57]

use "failures.length == 0" here and you don't need |success|

[James Cook, 2014/05/02 23:59:15]

Done.

+    chrome.test.notifyPass();
+  } else {
+    console.log("failures on:\n" + failures.join("\n") +
+        "\n\n\n>>> See comment in stubs_apitest.cc for a " +
+        "hint about fixing this failure.\n\n");
+    chrome.test.notifyFail("failed");
+  }
+}
+
+chrome.app.runtime.onLaunched.addListener(function() {
+  doTest();
+});