[Extensions] Improve app custom bindings

The app custom bindings (and handler counterparts) had issues. Fix some
of them.

BUG=None

Review-Url: https://codereview.chromium.org/2859233003
Cr-Commit-Position: refs/heads/master@{#471085}

chrome/renderer/extensions/app_bindings.cc

@@ -6,13 +6,7 @@
 
 #include <memory>
 
-#include "base/command_line.h"
-#include "base/macros.h"
-#include "base/strings/string16.h"
-#include "base/strings/string_util.h"
-#include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "content/public/child/v8_value_converter.h"
 #include "content/public/renderer/render_frame.h"
@@ -20,9 +14,7 @@
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/manifest.h"
-#include "extensions/renderer/console.h"
 #include "extensions/renderer/dispatcher.h"
-#include "extensions/renderer/extension_helper.h"
 #include "extensions/renderer/renderer_extension_registry.h"
 #include "extensions/renderer/script_context.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
@@ -34,12 +26,6 @@ using content::V8ValueConverter;
 
 namespace extensions {
 
-namespace {
-
-const char kInvalidCallbackIdError[] = "Invalid callbackId";
-
-}  // namespace
-
 AppBindings::AppBindings(Dispatcher* dispatcher, ScriptContext* context)
     : ObjectBackedNativeHandler(context),
       dispatcher_(dispatcher) {
@@ -77,7 +63,7 @@ void AppBindings::GetDetails(
 }
 
 v8::Local<v8::Value> AppBindings::GetDetailsImpl(blink::WebLocalFrame* frame) {
-  v8::Isolate* isolate = frame->MainWorldScriptContext()->GetIsolate();
+  v8::Isolate* isolate = context()->isolate();
   if (frame->GetDocument().GetSecurityOrigin().IsUnique())
     return v8::Null(isolate);
 
@@ -92,22 +78,15 @@ v8::Local<v8::Value> AppBindings::GetDetailsImpl(blink::WebLocalFrame* frame) {
       extension->manifest()->value()->DeepCopy());
   manifest_copy->SetString("id", extension->id());
   std::unique_ptr<V8ValueConverter> converter(V8ValueConverter::create());
-  return converter->ToV8Value(manifest_copy.get(),
-                              frame->MainWorldScriptContext());
+  return converter->ToV8Value(manifest_copy.get(), context()->v8_context());
 }
 
 void AppBindings::GetInstallState(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   // Get the callbackId.
-  int callback_id = 0;
-  if (args.Length() == 1) {
-    if (!args[0]->IsInt32()) {
-      context()->isolate()->ThrowException(v8::String::NewFromUtf8(
-          context()->isolate(), kInvalidCallbackIdError));
-      return;
-    }
-    callback_id = args[0]->Int32Value();
-  }
+  CHECK_EQ(1, args.Length());
+  CHECK(args[0]->IsInt32());
+  int callback_id = args[0]->Int32Value();
 
   content::RenderFrame* render_frame = context()->GetRenderFrame();
   CHECK(render_frame);
@@ -119,16 +98,15 @@ void AppBindings::GetInstallState(
 
 void AppBindings::GetRunningState(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
+  url::Origin top_origin = context()->web_frame()->Top()->GetSecurityOrigin();
+
   // To distinguish between ready_to_run and cannot_run states, we need the app
   // from the top frame.
-  blink::WebSecurityOrigin top_frame_security_origin =
-      context()->web_frame()->Top()->GetSecurityOrigin();
   const RendererExtensionRegistry* extensions =
       RendererExtensionRegistry::Get();
 
   // The app associated with the top level frame.
-  const Extension* top_app = extensions->GetHostedAppByURL(
-      GURL(top_frame_security_origin.ToString().Utf8()));
+  const Extension* top_app = extensions->GetHostedAppByURL(top_origin.GetURL());
 
   // The app associated with this frame.
   const Extension* this_app = extensions->GetHostedAppByURL(

chrome/renderer/extensions/app_bindings.h

@@ -11,11 +11,13 @@
 #ifndef CHROME_RENDERER_EXTENSIONS_APP_BINDINGS_H_
 #define CHROME_RENDERER_EXTENSIONS_APP_BINDINGS_H_
 
-#include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "chrome/renderer/extensions/chrome_v8_extension_handler.h"
 #include "extensions/renderer/object_backed_native_handler.h"
-#include "third_party/WebKit/public/web/WebFrame.h"
+
+namespace blink {
+class WebLocalFrame;
+}
 
 namespace extensions {
 class Dispatcher;

chrome/renderer/extensions/chrome_extensions_dispatcher_delegate.cc

@@ -42,6 +42,7 @@
 #include "extensions/renderer/script_context.h"
 #include "media/media_features.h"
 #include "third_party/WebKit/public/platform/WebString.h"
+#include "third_party/WebKit/public/platform/WebURL.h"
 #include "third_party/WebKit/public/web/WebSecurityPolicy.h"
 
 #if BUILDFLAG(ENABLE_WEBRTC)

chrome/renderer/resources/extensions/app_custom_bindings.js

@@ -35,13 +35,21 @@ var app = {
 //
 // So, define it manually, and let the getIsInstalled function act as its
 // documentation.
-app.__defineGetter__('isInstalled', wrapForLogging(appNatives.GetIsInstalled));
+var isInstalled = wrapForLogging(appNatives.GetIsInstalled);
+$Object.defineProperty(
+    app, 'isInstalled',
+    {
+      __proto__: null,
+      configurable: true,
+      enumerable: true,
+      get: function() { return isInstalled(); },
+    });
 
 // Called by app_bindings.cc.
 function onInstallStateResponse(state, callbackId) {
   var callback = callbacks[callbackId];
   delete callbacks[callbackId];
-  if (typeof(callback) == 'function') {
+  if (typeof callback == 'function') {
     try {
       callback(state);
     } catch (e) {
@@ -52,16 +60,24 @@ function onInstallStateResponse(state, callbackId) {
 }
 
 // TODO(kalman): move this stuff to its own custom bindings.
-var callbacks = {};
+var callbacks = { __proto__: null };
 var nextCallbackId = 1;
 
-app.installState = function getInstallState(callback) {
+function getInstallState(callback) {
   var callbackId = nextCallbackId++;
   callbacks[callbackId] = callback;
   appNatives.GetInstallState(callbackId);
-};
-if (extensionId)
-  app.installState = wrapForLogging(app.installState);
+}
+
+$Object.defineProperty(
+    app, 'installState',
+    {
+      __proto__: null,
+      configurable: true,
+      enumerable: true,
+      value: wrapForLogging(getInstallState),
+      writable: true,
+    });
 
 exports.$set('binding', app);
 exports.$set('onInstallStateResponse', onInstallStateResponse);