[Extensions Bindings] Check availability of custom API properties

extensions/renderer/bindings/api_binding.cc

Index: extensions/renderer/bindings/api_binding.cc
diff --git a/extensions/renderer/bindings/api_binding.cc b/extensions/renderer/bindings/api_binding.cc
index e69eccab892f4b353b29b5c5192d380ccfb0b138..53c963b2eff6256337844969ad3611b571710354 100644
--- a/extensions/renderer/bindings/api_binding.cc
+++ b/extensions/renderer/bindings/api_binding.cc
@@ -30,6 +30,21 @@ namespace extensions {
 
 namespace {
 
+std::string GetPlatformString() {
+#if defined(OS_CHROMEOS)
+  return "chromeos";

[jbroman, 2017/06/29 15:24:21]

I assume you'll look into what's up with protectedContentEnabled referring to
"cros" and "cros touch", which aren't used here or in binding.js?

[Devlin, 2017/06/29 21:00:51]

Yep!  This just puts it more inline with the end result of the current bindings
and uses the same enums as the features files.

+#elif defined(OS_LINUX)
+  return "linux";
+#elif defined(OS_MACOSX)
+  return "mac";
+#elif defined(OS_WIN)
+  return "win";
+#else
+  NOTREACHED();
+  return std::string();
+#endif
+}
+
 // Returns the name of the enum value for use in JavaScript; JS enum entries use
 // SCREAMING_STYLE.
 std::string GetJSEnumEntryName(const std::string& original) {
@@ -428,6 +443,28 @@ void APIBinding::DecorateTemplateWithProperties(
       continue;
     }
 
+    const base::ListValue* platforms = nullptr;
+    // TODO(devlin): This isn't great. It's bad to have availability primarily
+    // defined in the features files, and then partially defined within the
+    // API specification itself. Additionally, they aren't equivalent
+    // definitions. But given the rarity of property restrictions, and the fact
+    // that they are all limited by platform, it makes more sense to isolate
+    // this check here. If this becomes more common, we should really find a
+    // way of moving these checks to the features files.
+    if (dict->GetList("platforms", &platforms)) {
+      std::string platform_string = GetPlatformString();
+      bool allowed = false;
+      for (const auto& platform : *platforms) {

[jbroman, 2017/06/29 15:24:20]

super-nit: your choice, but I mildly prefer <algorithm> style here:

std::string this_platform = GetPlatformString();
auto is_this_platform = [&this_platform](const base::Value& platform) {
  return platform.is_string() && platform.GetString() == this_platform;
};
if (std::none_of(platforms->begin(), platforms->end(), is_this_platform))
  continue;

[Devlin, 2017/06/29 21:00:51]

Oh, forgot about none_of.  I had a version that used find_if(...) != end(), but
that was clunkier than the for loop.  This is nice, though.

Done.

+        if (platform.is_string() && platform.GetString() == platform_string) {
+          allowed = true;
+          break;
+        }
+      }
+
+      if (!allowed)
+        continue;
+    }
+
     v8::Local<v8::String> v8_key = gin::StringToSymbol(isolate, iter.key());
     std::string ref;
     if (dict->GetString("$ref", &ref)) {