summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorpmarch@chromium.org <pmarch@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-08-12 17:51:36 +0000
committerpmarch@chromium.org <pmarch@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-08-12 17:51:36 +0000
commitd754cbb0f9c177e9e881e2dc2e3b721c4a1b7048 (patch)
treec84a4b1fe2893ef510fd0825993c248d0f8124de
parent581de1f37dd528c558ad5cca2fae0d95cd7c07fe (diff)
downloadchromium_src-d754cbb0f9c177e9e881e2dc2e3b721c4a1b7048.zip
chromium_src-d754cbb0f9c177e9e881e2dc2e3b721c4a1b7048.tar.gz
chromium_src-d754cbb0f9c177e9e881e2dc2e3b721c4a1b7048.tar.bz2
V8ValueConverter for the activity logger that does not invoke interceptors and
accessors when converting V8 values to base values. BUG=260978, 259093 Review URL: https://chromiumcodereview.appspot.com/19730002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@217032 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/chrome_renderer.gypi2
-rw-r--r--chrome/chrome_tests_unit.gypi1
-rw-r--r--chrome/renderer/extensions/activity_log_converter_strategy.cc54
-rw-r--r--chrome/renderer/extensions/activity_log_converter_strategy.h36
-rw-r--r--chrome/renderer/extensions/activity_log_converter_strategy_unittest.cc158
-rw-r--r--chrome/renderer/extensions/api_activity_logger.cc4
-rw-r--r--chrome/renderer/extensions/dom_activity_logger.cc4
-rw-r--r--chrome/test/data/extensions/api_test/activity_log_private/test/test.js5
-rw-r--r--content/public/renderer/v8_value_converter.h17
-rw-r--r--content/renderer/v8_value_converter_impl.cc20
-rw-r--r--content/renderer/v8_value_converter_impl.h4
11 files changed, 299 insertions, 6 deletions
diff --git a/chrome/chrome_renderer.gypi b/chrome/chrome_renderer.gypi
index ccb508e..5b0d88c 100644
--- a/chrome/chrome_renderer.gypi
+++ b/chrome/chrome_renderer.gypi
@@ -49,6 +49,8 @@
'sources': [
'renderer/benchmarking_extension.cc',
'renderer/benchmarking_extension.h',
+ 'renderer/extensions/activity_log_converter_strategy.cc',
+ 'renderer/extensions/activity_log_converter_strategy.h',
'renderer/extensions/api_activity_logger.cc',
'renderer/extensions/api_activity_logger.h',
'renderer/extensions/api_definitions_natives.cc',
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi
index e2dda05..6fdcd3c 100644
--- a/chrome/chrome_tests_unit.gypi
+++ b/chrome/chrome_tests_unit.gypi
@@ -1786,6 +1786,7 @@
'common/worker_thread_ticker_unittest.cc',
'renderer/chrome_content_renderer_client_unittest.cc',
'renderer/content_settings_observer_unittest.cc',
+ 'renderer/extensions/activity_log_converter_strategy_unittest.cc',
'renderer/extensions/chrome_v8_context_set_unittest.cc',
'renderer/extensions/event_unittest.cc',
'renderer/extensions/extension_localization_peer_unittest.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 0000000..1c5906e
--- /dev/null
+++ b/chrome/renderer/extensions/activity_log_converter_strategy.cc
@@ -0,0 +1,54 @@
+// Copyright 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->GetConstructorName());
+ }
+ 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
diff --git a/chrome/renderer/extensions/activity_log_converter_strategy.h b/chrome/renderer/extensions/activity_log_converter_strategy.h
new file mode 100644
index 0000000..aa5af28
--- /dev/null
+++ b/chrome/renderer/extensions/activity_log_converter_strategy.h
@@ -0,0 +1,36 @@
+// Copyright 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.
+
+#ifndef CHROME_RENDERER_EXTENSIONS_ACTIVITY_LOG_CONVERTER_STRATEGY_H_
+#define CHROME_RENDERER_EXTENSIONS_ACTIVITY_LOG_CONVERTER_STRATEGY_H_
+
+#include "base/compiler_specific.h"
+#include "content/public/renderer/v8_value_converter.h"
+#include "v8/include/v8.h"
+
+namespace extensions {
+
+// This class is used by activity logger and extends behavior of
+// V8ValueConverter. It overwrites conversion of V8 arrays and objects. When
+// converting arrays and objects, we must not invoke any JS code which may
+// result in triggering activity logger. In such case, the log entries will be
+// generated due to V8 object conversion rather than extension activity.
+class ActivityLogConverterStrategy
+ : public content::V8ValueConverter::Strategy {
+ public:
+ virtual ~ActivityLogConverterStrategy() {}
+
+ virtual bool FromV8Object(v8::Handle<v8::Object> value,
+ base::Value** out) const OVERRIDE;
+ virtual bool FromV8Array(v8::Handle<v8::Array> value,
+ base::Value** out) const OVERRIDE;
+
+ private:
+ bool FromV8ObjectInternal(v8::Handle<v8::Object> value,
+ base::Value** out) const;
+};
+
+} // namespace extensions
+
+#endif // CHROME_RENDERER_EXTENSIONS_ACTIVITY_LOG_CONVERTER_STRATEGY_H_
diff --git a/chrome/renderer/extensions/activity_log_converter_strategy_unittest.cc b/chrome/renderer/extensions/activity_log_converter_strategy_unittest.cc
new file mode 100644
index 0000000..b113d7a
--- /dev/null
+++ b/chrome/renderer/extensions/activity_log_converter_strategy_unittest.cc
@@ -0,0 +1,158 @@
+// Copyright 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 "base/memory/scoped_ptr.h"
+#include "base/values.h"
+#include "chrome/renderer/extensions/activity_log_converter_strategy.h"
+#include "chrome/renderer/extensions/scoped_persistent.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "v8/include/v8.h"
+
+using content::V8ValueConverter;
+
+namespace extensions {
+
+class ActivityLogConverterStrategyTest : public testing::Test {
+ public:
+ ActivityLogConverterStrategyTest()
+ : isolate_(v8::Isolate::GetCurrent())
+ , handle_scope_(isolate_)
+ , context_(v8::Context::New(isolate_))
+ , context_scope_(context_.get()) {
+ }
+
+ protected:
+ virtual void SetUp() {
+ converter_.reset(V8ValueConverter::create());
+ strategy_.reset(new ActivityLogConverterStrategy());
+ converter_->SetFunctionAllowed(true);
+ converter_->SetStrategy(strategy_.get());
+ }
+
+ testing::AssertionResult VerifyNull(v8::Local<v8::Value> v8_value) {
+ scoped_ptr<base::Value> value(
+ converter_->FromV8Value(v8_value, context_.get()));
+ if (value->IsType(base::Value::TYPE_NULL))
+ return testing::AssertionSuccess();
+ return testing::AssertionFailure();
+ }
+
+ testing::AssertionResult VerifyBoolean(v8::Local<v8::Value> v8_value,
+ bool expected) {
+ bool out;
+ scoped_ptr<base::Value> value(
+ converter_->FromV8Value(v8_value, context_.get()));
+ if (value->IsType(base::Value::TYPE_BOOLEAN)
+ && value->GetAsBoolean(&out)
+ && out == expected)
+ return testing::AssertionSuccess();
+ return testing::AssertionFailure();
+ }
+
+ testing::AssertionResult VerifyInteger(v8::Local<v8::Value> v8_value,
+ int expected) {
+ int out;
+ scoped_ptr<base::Value> value(
+ converter_->FromV8Value(v8_value, context_.get()));
+ if (value->IsType(base::Value::TYPE_INTEGER)
+ && value->GetAsInteger(&out)
+ && out == expected)
+ return testing::AssertionSuccess();
+ return testing::AssertionFailure();
+ }
+
+ testing::AssertionResult VerifyDouble(v8::Local<v8::Value> v8_value,
+ double expected) {
+ double out;
+ scoped_ptr<base::Value> value(
+ converter_->FromV8Value(v8_value, context_.get()));
+ if (value->IsType(base::Value::TYPE_DOUBLE)
+ && value->GetAsDouble(&out)
+ && out == expected)
+ return testing::AssertionSuccess();
+ return testing::AssertionFailure();
+ }
+
+ testing::AssertionResult VerifyString(v8::Local<v8::Value> v8_value,
+ const std::string& expected) {
+ std::string out;
+ scoped_ptr<base::Value> value(
+ converter_->FromV8Value(v8_value, context_.get()));
+ if (value->IsType(base::Value::TYPE_STRING)
+ && value->GetAsString(&out)
+ && out == expected)
+ return testing::AssertionSuccess();
+ return testing::AssertionFailure();
+ }
+
+ v8::Isolate* isolate_;
+ v8::HandleScope handle_scope_;
+ ScopedPersistent<v8::Context> context_;
+ v8::Context::Scope context_scope_;
+ scoped_ptr<V8ValueConverter> converter_;
+ scoped_ptr<ActivityLogConverterStrategy> strategy_;
+};
+
+TEST_F(ActivityLogConverterStrategyTest, ConversionTest) {
+ const char* source = "(function() {"
+ "function foo() {}"
+ "return {"
+ "null: null,"
+ "true: true,"
+ "false: false,"
+ "positive_int: 42,"
+ "negative_int: -42,"
+ "zero: 0,"
+ "double: 88.8,"
+ "big_integral_double: 9007199254740992.0," // 2.0^53
+ "string: \"foobar\","
+ "empty_string: \"\","
+ "dictionary: {"
+ "foo: \"bar\","
+ "hot: \"dog\","
+ "},"
+ "empty_dictionary: {},"
+ "list: [ \"monkey\", \"balls\" ],"
+ "empty_list: [],"
+ "function: function() {},"
+ "named_function: foo"
+ "};"
+ "})();";
+
+ v8::Handle<v8::Script> script(v8::Script::New(v8::String::New(source)));
+ v8::Handle<v8::Object> v8_object = script->Run().As<v8::Object>();
+
+ EXPECT_TRUE(VerifyString(v8_object, "[Object]"));
+ EXPECT_TRUE(VerifyNull(v8_object->Get(v8::String::New("null"))));
+ EXPECT_TRUE(VerifyBoolean(v8_object->Get(v8::String::New("true")), true));
+ EXPECT_TRUE(VerifyBoolean(v8_object->Get(v8::String::New("false")), false));
+ EXPECT_TRUE(VerifyInteger(v8_object->Get(v8::String::New("positive_int")),
+ 42));
+ EXPECT_TRUE(VerifyInteger(v8_object->Get(v8::String::New("negative_int")),
+ -42));
+ EXPECT_TRUE(VerifyInteger(v8_object->Get(v8::String::New("zero")), 0));
+ EXPECT_TRUE(VerifyDouble(v8_object->Get(v8::String::New("double")), 88.8));
+ EXPECT_TRUE(VerifyDouble(
+ v8_object->Get(v8::String::New("big_integral_double")),
+ 9007199254740992.0));
+ EXPECT_TRUE(VerifyString(v8_object->Get(v8::String::New("string")),
+ "foobar"));
+ EXPECT_TRUE(VerifyString(v8_object->Get(v8::String::New("empty_string")),
+ ""));
+ EXPECT_TRUE(VerifyString(v8_object->Get(v8::String::New("dictionary")),
+ "[Object]"));
+ EXPECT_TRUE(VerifyString(v8_object->Get(v8::String::New("empty_dictionary")),
+ "[Object]"));
+ EXPECT_TRUE(VerifyString(v8_object->Get(v8::String::New("list")),
+ "[Array]"));
+ EXPECT_TRUE(VerifyString(v8_object->Get(v8::String::New("empty_list")),
+ "[Array]"));
+ EXPECT_TRUE(VerifyString(v8_object->Get(v8::String::New("function")),
+ "[Function]"));
+ EXPECT_TRUE(VerifyString(v8_object->Get(v8::String::New("named_function")),
+ "[Function foo()]"));
+}
+
+} // namespace extensions
+
diff --git a/chrome/renderer/extensions/api_activity_logger.cc b/chrome/renderer/extensions/api_activity_logger.cc
index 28a4367..a6b95af 100644
--- a/chrome/renderer/extensions/api_activity_logger.cc
+++ b/chrome/renderer/extensions/api_activity_logger.cc
@@ -6,6 +6,7 @@
#include "base/bind.h"
#include "chrome/common/extensions/extension_messages.h"
#include "chrome/renderer/chrome_render_process_observer.h"
+#include "chrome/renderer/extensions/activity_log_converter_strategy.h"
#include "chrome/renderer/extensions/api_activity_logger.h"
#include "content/public/renderer/render_thread.h"
#include "content/public/renderer/v8_value_converter.h"
@@ -56,6 +57,9 @@ void APIActivityLogger::LogInternal(
v8::Local<v8::Array> arg_array = v8::Local<v8::Array>::Cast(args[2]);
if (arg_array->Length() > 0) {
scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
+ ActivityLogConverterStrategy strategy;
+ converter->SetFunctionAllowed(true);
+ converter->SetStrategy(&strategy);
scoped_ptr<ListValue> arg_list(new ListValue());
for (size_t i = 0; i < arg_array->Length(); ++i) {
arg_list->Set(i,
diff --git a/chrome/renderer/extensions/dom_activity_logger.cc b/chrome/renderer/extensions/dom_activity_logger.cc
index 95d1c11..3c7a8cf 100644
--- a/chrome/renderer/extensions/dom_activity_logger.cc
+++ b/chrome/renderer/extensions/dom_activity_logger.cc
@@ -8,6 +8,7 @@
#include "chrome/common/extensions/dom_action_types.h"
#include "chrome/common/extensions/extension_messages.h"
#include "chrome/renderer/chrome_render_process_observer.h"
+#include "chrome/renderer/extensions/activity_log_converter_strategy.h"
#include "content/public/renderer/render_thread.h"
#include "content/public/renderer/v8_value_converter.h"
#include "third_party/WebKit/public/platform/WebString.h"
@@ -30,6 +31,9 @@ void DOMActivityLogger::log(
const v8::Handle<v8::Value> argv[],
const WebString& call_type) {
scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
+ ActivityLogConverterStrategy strategy;
+ converter->SetFunctionAllowed(true);
+ converter->SetStrategy(&strategy);
scoped_ptr<ListValue> argv_list_value(new ListValue());
for (int i =0; i < argc; i++) {
argv_list_value->Set(
diff --git a/chrome/test/data/extensions/api_test/activity_log_private/test/test.js b/chrome/test/data/extensions/api_test/activity_log_private/test/test.js
index 0143cce..e1572a0 100644
--- a/chrome/test/data/extensions/api_test/activity_log_private/test/test.js
+++ b/chrome/test/data/extensions/api_test/activity_log_private/test/test.js
@@ -217,13 +217,8 @@ domExpectedActivity = [
// Dom mutations
'Document.createElement',
'Document.createElement',
- 'Document.location',
'Node.appendChild',
- 'Document.location',
- 'Document.location',
'Node.insertBefore',
- 'Document.location',
- 'Document.location',
'Node.replaceChild',
//'Document.location',
'HTMLDocument.write',
diff --git a/content/public/renderer/v8_value_converter.h b/content/public/renderer/v8_value_converter.h
index b71b32d..11514f7 100644
--- a/content/public/renderer/v8_value_converter.h
+++ b/content/public/renderer/v8_value_converter.h
@@ -24,6 +24,20 @@ namespace content {
// ArrayBufferView subclasses (Uint8Array, etc.).
class CONTENT_EXPORT V8ValueConverter {
public:
+ // Extends the default behaviour of V8ValueConverter.
+ class CONTENT_EXPORT Strategy {
+ public:
+ virtual ~Strategy() {}
+ // If false is returned, V8ValueConverter proceeds with the default
+ // behavior.
+ virtual bool FromV8Object(v8::Handle<v8::Object> value,
+ base::Value** out) const = 0;
+ // If false is returned, V8ValueConverter proceeds with the default
+ // behavior.
+ virtual bool FromV8Array(v8::Handle<v8::Array> value,
+ base::Value** out) const = 0;
+ };
+
static V8ValueConverter* create();
virtual ~V8ValueConverter() {}
@@ -52,6 +66,9 @@ class CONTENT_EXPORT V8ValueConverter {
// converting arguments to extension APIs.
virtual void SetStripNullFromObjects(bool val) = 0;
+ // Extend default behavior of V8ValueConverter.
+ virtual void SetStrategy(Strategy* strategy) = 0;
+
// Converts a base::Value to a v8::Value.
//
// Unsupported types are replaced with null. If an array or object throws
diff --git a/content/renderer/v8_value_converter_impl.cc b/content/renderer/v8_value_converter_impl.cc
index cb4b9af..efd3d44 100644
--- a/content/renderer/v8_value_converter_impl.cc
+++ b/content/renderer/v8_value_converter_impl.cc
@@ -86,7 +86,8 @@ V8ValueConverterImpl::V8ValueConverterImpl()
reg_exp_allowed_(false),
function_allowed_(false),
strip_null_from_objects_(false),
- avoid_identity_hash_for_testing_(false) {}
+ avoid_identity_hash_for_testing_(false),
+ strategy_(NULL) {}
void V8ValueConverterImpl::SetDateAllowed(bool val) {
date_allowed_ = val;
@@ -104,6 +105,10 @@ void V8ValueConverterImpl::SetStripNullFromObjects(bool val) {
strip_null_from_objects_ = val;
}
+void V8ValueConverterImpl::SetStrategy(Strategy* strategy) {
+ strategy_ = strategy;
+}
+
v8::Handle<v8::Value> V8ValueConverterImpl::ToV8Value(
const base::Value* value, v8::Handle<v8::Context> context) const {
v8::Context::Scope context_scope(context);
@@ -302,6 +307,12 @@ base::Value* V8ValueConverterImpl::FromV8Array(
val->CreationContext() != v8::Context::GetCurrent())
scope.reset(new v8::Context::Scope(val->CreationContext()));
+ if (strategy_) {
+ Value* out = NULL;
+ if (strategy_->FromV8Array(val, &out))
+ return out;
+ }
+
base::ListValue* result = new base::ListValue();
// Only fields with integer keys are carried over to the ListValue.
@@ -357,6 +368,7 @@ base::Value* V8ValueConverterImpl::FromV8Object(
FromV8ValueState* state) const {
if (!state->UpdateAndCheckUniqueness(val))
return base::Value::CreateNullValue();
+
scoped_ptr<v8::Context::Scope> scope;
// If val was created in a different context than our current one, change to
// that context, but change back after val is converted.
@@ -364,6 +376,12 @@ base::Value* V8ValueConverterImpl::FromV8Object(
val->CreationContext() != v8::Context::GetCurrent())
scope.reset(new v8::Context::Scope(val->CreationContext()));
+ if (strategy_) {
+ Value* out = NULL;
+ if (strategy_->FromV8Object(val, &out))
+ return out;
+ }
+
scoped_ptr<base::DictionaryValue> result(new base::DictionaryValue());
v8::Handle<v8::Array> property_names(val->GetOwnPropertyNames());
diff --git a/content/renderer/v8_value_converter_impl.h b/content/renderer/v8_value_converter_impl.h
index f5ad10a..2033164 100644
--- a/content/renderer/v8_value_converter_impl.h
+++ b/content/renderer/v8_value_converter_impl.h
@@ -30,6 +30,7 @@ class CONTENT_EXPORT V8ValueConverterImpl : public V8ValueConverter {
virtual void SetRegExpAllowed(bool val) OVERRIDE;
virtual void SetFunctionAllowed(bool val) OVERRIDE;
virtual void SetStripNullFromObjects(bool val) OVERRIDE;
+ virtual void SetStrategy(Strategy* strategy) OVERRIDE;
virtual v8::Handle<v8::Value> ToV8Value(
const base::Value* value,
v8::Handle<v8::Context> context) const OVERRIDE;
@@ -76,6 +77,9 @@ class CONTENT_EXPORT V8ValueConverterImpl : public V8ValueConverter {
bool avoid_identity_hash_for_testing_;
+ // Strategy object that changes the converter's behavior.
+ Strategy* strategy_;
+
DISALLOW_COPY_AND_ASSIGN(V8ValueConverterImpl);
};