[Extensions Bindings] Support chrome.extension.lastError

extensions/renderer/bindings/api_last_error.cc

Index: extensions/renderer/bindings/api_last_error.cc
diff --git a/extensions/renderer/bindings/api_last_error.cc b/extensions/renderer/bindings/api_last_error.cc
index 95c9cbdb90bf246ab9d2ee2c7f37267f4729a08e..660cab612b5a042a3605f96a3a3da4a6274a4da3 100644
--- a/extensions/renderer/bindings/api_last_error.cc
+++ b/extensions/renderer/bindings/api_last_error.cc
@@ -5,6 +5,7 @@
 #include "extensions/renderer/bindings/api_last_error.h"
 
 #include "gin/converter.h"
+#include "gin/data_object_builder.h"
 #include "gin/handle.h"
 #include "gin/object_template_builder.h"
 #include "gin/wrappable.h"
@@ -132,47 +133,11 @@ void APILastError::SetError(v8::Local<v8::Context> context,
   v8::TryCatch try_catch(isolate);
   try_catch.SetVerbose(true);
 
-  v8::Local<v8::Object> parent = get_parent_.Run(context);
-  if (parent.IsEmpty())
-    return;
-  v8::Local<v8::String> key = gin::StringToSymbol(isolate, kLastErrorProperty);
-  v8::Local<v8::Value> v8_error;
-  // Two notes: this Get() is visible to external script, and this will actually
-  // mark the lastError as accessed, if one exists. These shouldn't be a
-  // problem (lastError is meant to be helpful, but isn't designed to handle
-  // crazy chaining, etc). However, if we decide we needed to be fancier, we
-  // could detect the presence of a current error through a GetPrivate(), and
-  // optionally throw it if one exists.
-  if (!parent->Get(context, key).ToLocal(&v8_error))
-    return;
+  v8::Local<v8::Object> secondary_parent;
+  v8::Local<v8::Object> parent = get_parent_.Run(context, &secondary_parent);
 
-  if (!v8_error->IsUndefined()) {
-    // There may be an existing last error to overwrite.
-    LastErrorObject* last_error = nullptr;
-    if (!gin::Converter<LastErrorObject*>::FromV8(isolate, v8_error,
-                                                  &last_error)) {
-      // If it's not a real lastError (e.g. if a script manually set it), don't
-      // do anything. We shouldn't mangle a property set by other script.
-      // TODO(devlin): Or should we? If someone sets chrome.runtime.lastError,
-      // it might be the right course of action to overwrite it.
-      return;
-    }
-    last_error->Reset(error);
-  } else {
-    v8::Local<v8::Value> last_error =
-        gin::CreateHandle(isolate, new LastErrorObject(error)).ToV8();
-    v8::Maybe<bool> set_private = parent->SetPrivate(
-        context, v8::Private::ForApi(isolate, key), last_error);
-    if (!set_private.IsJust() || !set_private.FromJust()) {
-      NOTREACHED();
-      return;
-    }
-    DCHECK(!last_error.IsEmpty());
-    // This Set() can fail, but there's nothing to do if it does (the exception
-    // will be caught by the TryCatch above).
-    ignore_result(parent->SetAccessor(context, key, &LastErrorGetter,
-                                      &LastErrorSetter, last_error));
-  }
+  SetErrorOnPrimaryParent(context, parent, error);
+  SetErrorOnSecondaryParent(context, secondary_parent, error);
 }
 
 void APILastError::ClearError(v8::Local<v8::Context> context,
@@ -181,6 +146,7 @@ void APILastError::ClearError(v8::Local<v8::Context> context,
   v8::HandleScope handle_scope(isolate);
 
   v8::Local<v8::Object> parent;
+  v8::Local<v8::Object> secondary_parent;
   LastErrorObject* last_error = nullptr;
   v8::Local<v8::String> key;
   v8::Local<v8::Private> private_key;
@@ -189,16 +155,15 @@ void APILastError::ClearError(v8::Local<v8::Context> context,
     v8::TryCatch try_catch(isolate);
     try_catch.SetVerbose(true);
 
-    parent = get_parent_.Run(context);
+    parent = get_parent_.Run(context, &secondary_parent);
     if (parent.IsEmpty())
       return;
     key = gin::StringToSymbol(isolate, kLastErrorProperty);
     private_key = v8::Private::ForApi(isolate, key);
     v8::Local<v8::Value> error;
     // Access through GetPrivate() so that we don't trigger accessed().
-    if (!parent->GetPrivate(context, private_key).ToLocal(&error))
-      return;
-    if (!gin::Converter<LastErrorObject*>::FromV8(context->GetIsolate(), error,
+    if (!parent->GetPrivate(context, private_key).ToLocal(&error) ||
+        !gin::Converter<LastErrorObject*>::FromV8(context->GetIsolate(), error,
                                                   &last_error)) {
       return;
     }
@@ -218,9 +183,11 @@ void APILastError::ClearError(v8::Local<v8::Context> context,
     NOTREACHED();
     return;
   }
-  // This Delete() can fail, but there's nothing to do if it does (the exception
-  // will be caught by the TryCatch above).
+  // These Delete()s can fail, but there's nothing to do if it does (the
+  // exception will be caught by the TryCatch above).
   ignore_result(parent->Delete(context, key));
+  if (!secondary_parent.IsEmpty())
+    ignore_result(secondary_parent->Delete(context, key));
 }
 
 bool APILastError::HasError(v8::Local<v8::Context> context) {
@@ -231,7 +198,7 @@ bool APILastError::HasError(v8::Local<v8::Context> context) {
   v8::TryCatch try_catch(isolate);
   try_catch.SetVerbose(true);
 
-  v8::Local<v8::Object> parent = get_parent_.Run(context);
+  v8::Local<v8::Object> parent = get_parent_.Run(context, nullptr);
   if (parent.IsEmpty())
     return false;
   v8::Local<v8::Value> error;
@@ -246,4 +213,70 @@ bool APILastError::HasError(v8::Local<v8::Context> context) {
                                                   &last_error);
 }
 
+void APILastError::SetErrorOnPrimaryParent(v8::Local<v8::Context> context,
+                                           v8::Local<v8::Object> parent,
+                                           const std::string& error) {
+  if (parent.IsEmpty())
+    return;
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::Local<v8::String> key = gin::StringToSymbol(isolate, kLastErrorProperty);
+  v8::Local<v8::Value> v8_error;
+  // Two notes: this Get() is visible to external script, and this will actually
+  // mark the lastError as accessed, if one exists. These shouldn't be a
+  // problem (lastError is meant to be helpful, but isn't designed to handle
+  // crazy chaining, etc). However, if we decide we needed to be fancier, we
+  // could detect the presence of a current error through a GetPrivate(), and
+  // optionally throw it if one exists.
+  if (!parent->Get(context, key).ToLocal(&v8_error))
+    return;
+
+  if (!v8_error->IsUndefined()) {
+    // There may be an existing last error to overwrite.
+    LastErrorObject* last_error = nullptr;
+    if (!gin::Converter<LastErrorObject*>::FromV8(isolate, v8_error,
+                                                  &last_error)) {
+      // If it's not a real lastError (e.g. if a script manually set it), don't
+      // do anything. We shouldn't mangle a property set by other script.
+      // TODO(devlin): Or should we? If someone sets chrome.runtime.lastError,
+      // it might be the right course of action to overwrite it.
+      return;
+    }
+    last_error->Reset(error);
+  } else {
+    v8::Local<v8::Value> last_error =
+        gin::CreateHandle(isolate, new LastErrorObject(error)).ToV8();
+    v8::Maybe<bool> set_private = parent->SetPrivate(
+        context, v8::Private::ForApi(isolate, key), last_error);
+    if (!set_private.IsJust() || !set_private.FromJust()) {
+      NOTREACHED();
+      return;
+    }
+    DCHECK(!last_error.IsEmpty());
+    // This SetAccessor() can fail, but there's nothing to do if it does (the
+    // exception will be caught by the TryCatch in SetError()).
+    ignore_result(parent->SetAccessor(context, key, &LastErrorGetter,
+                                      &LastErrorSetter, last_error));
+  }
+}
+
+void APILastError::SetErrorOnSecondaryParent(
+    v8::Local<v8::Context> context,
+    v8::Local<v8::Object> secondary_parent,
+    const std::string& error) {
+  if (secondary_parent.IsEmpty())
+    return;
+
+  // For the secondary parent, simply set chrome.extension.lastError to
+  // {message: <error>}.
+  // TODO(devlin): Gather metrics on how frequently this is checked. It'd be
+  // nice to get rid of it.
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::Local<v8::String> key = gin::StringToSymbol(isolate, kLastErrorProperty);
+  // This CreateDataProperty() can fail, but there's nothing to do if it does
+  // (the exception will be caught by the TryCatch in SetError()).
+  ignore_result(secondary_parent->CreateDataProperty(
+      context, key,
+      gin::DataObjectBuilder(isolate).Set("message", error).Build()));
+}
+
 }  // namespace extensions