V8ValueConverter for the activity logger that does not invoke interceptors and

chrome/renderer/extensions/activity_log_converter_strategy.cc

Index: chrome/renderer/extensions/activity_log_converter_strategy.cc
diff --git a/chrome/renderer/extensions/activity_log_converter_strategy.cc b/chrome/renderer/extensions/activity_log_converter_strategy.cc
new file mode 100644
index 0000000000000000000000000000000000000000..aef1b15032469bc9cb6e16da339abd458ac88192
--- /dev/null
+++ b/chrome/renderer/extensions/activity_log_converter_strategy.cc
@@ -0,0 +1,54 @@
+// 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.
+
+#include "chrome/renderer/extensions/activity_log_converter_strategy.h"
+#include "base/values.h"
+
+namespace extensions {
+
+bool ActivityLogConverterStrategy::FromV8Object(v8::Handle<v8::Object> value,
+                                                base::Value** out) const {
+  return FromV8ObjectInternal(value, out);
+}
+
+bool ActivityLogConverterStrategy::FromV8Array(v8::Handle<v8::Array> value,
+                                               base::Value** out) const {
+  return FromV8ObjectInternal(value, out);
+}
+
+bool ActivityLogConverterStrategy::FromV8ObjectInternal(
+    v8::Handle<v8::Object> value,
+    base::Value** out) const {
+
+  // Handle JSObject.
+  // We cannot use value->Get(key/index) as there may be a getter method,
+  // accessor, interceptor/handler, or access check callback set on the
+  // property. If that it is the case, any of those may invoke JS code that
+  // may result on logging extension activity events caused by value conversion
+  // rather than extension itself.
+
+  // V8 arrays are handled here in the same way as other JSObjects as they may
+  // also have getter methods, accessor, interceptor/handler, and access check
+  // callback.
+
+  v8::Handle<v8::String> name = v8::String::New("[");
+  if (value->IsFunction()) {
+    name =  v8::String::Concat(name, v8::String::New("Function"));
+    v8::Handle<v8::Value> fname =
+        v8::Handle<v8::Function>::Cast(value)->GetName();
+    if (fname->IsString() && v8::Handle<v8::String>::Cast(fname)->Length()) {
+      name = v8::String::Concat(name, v8::String::New(" "));
+      name = v8::String::Concat(name, v8::Handle<v8::String>::Cast(fname));
+      name = v8::String::Concat(name, v8::String::New("()"));
+    }
+  } else {
+    name = v8::String::Concat(name, value->ToObject()->GetConstructorName());

[dcarney, 2013/08/12 08:47:04]

Do you want ToObject?  This is relatively expensive compared to the other things
happening here.  An IsObject check and a Cast would be more inline with the code
for function above in terms of cost, but I have no idea what the goal here is.

[pmarch, 2013/08/12 13:39:26]

Thanks Dan, nice catch. TOObject is absolutely redundant here.

+  }
+  name = v8::String::Concat(name, v8::String::New("]"));
+  *out = new base::StringValue(std::string(*v8::String::Utf8Value(name)));
+  // Prevent V8ValueConverter from further processing this object.
+  return true;
+}
+
+}  // namespace extensions