Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(319)

Unified Diff: extensions/renderer/console.cc

Issue 2718133003: Rewrite extensions console to not leak function templates. (Closed)
Patch Set: devlin Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « extensions/renderer/console.h ('k') | extensions/renderer/module_system.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: extensions/renderer/console.cc
diff --git a/extensions/renderer/console.cc b/extensions/renderer/console.cc
index 30e42eb34b56ac5ac7c34a1ae12d1c4710b2fcae..245e14f9c24bef22fd58a154884b4f1cfaa2cbd3 100644
--- a/extensions/renderer/console.cc
+++ b/extensions/renderer/console.cc
@@ -17,6 +17,8 @@
#include "extensions/renderer/script_context.h"
#include "extensions/renderer/script_context_set.h"
#include "extensions/renderer/v8_helpers.h"
+#include "gin/converter.h"
+#include "gin/per_isolate_data.h"
namespace extensions {
namespace console {
@@ -34,9 +36,6 @@ void CheckWithMinidump(const std::string& message) {
CHECK(false) << message;
}
-typedef void (*LogMethod)(content::RenderFrame* render_frame,
- const std::string& message);
-
void BoundLogMethodCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
std::string message;
for (int i = 0; i < info.Length(); ++i) {
@@ -45,68 +44,29 @@ void BoundLogMethodCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
message += *v8::String::Utf8Value(info[i]);
}
- v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
- if (context.IsEmpty()) {
- LOG(WARNING) << "Could not log \"" << message << "\": no context given";
- return;
- }
-
// A worker's ScriptContext neither lives in ScriptContextSet nor it has a
// RenderFrame associated with it, so early exit in this case.
// TODO(lazyboy): Fix.
if (content::WorkerThread::GetCurrentId() > 0)
return;
+ v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
ScriptContext* script_context =
ScriptContextSet::GetContextByV8Context(context);
- LogMethod log_method =
- reinterpret_cast<LogMethod>(info.Data().As<v8::External>()->Value());
- (*log_method)(script_context ? script_context->GetRenderFrame() : nullptr,
- message);
+ // TODO(devlin): Consider (D)CHECK(script_context)
+ content::RenderFrame* render_frame =
+ script_context ? script_context->GetRenderFrame() : nullptr;
+ const auto level = static_cast<content::ConsoleMessageLevel>(
+ info.Data().As<v8::Int32>()->Value());
+ AddMessage(render_frame, level, message);
}
-void BindLogMethod(v8::Isolate* isolate,
- v8::Local<v8::Object> target,
- const std::string& name,
- LogMethod log_method) {
- v8::Local<v8::FunctionTemplate> tmpl = v8::FunctionTemplate::New(
- isolate,
- &BoundLogMethodCallback,
- v8::External::New(isolate, reinterpret_cast<void*>(log_method)));
- tmpl->RemovePrototype();
- v8::Local<v8::Function> function;
- if (!tmpl->GetFunction(isolate->GetCurrentContext()).ToLocal(&function)) {
- LOG(FATAL) << "Could not create log function \"" << name << "\"";
- return;
- }
- v8::Local<v8::String> v8_name = ToV8StringUnsafe(isolate, name);
- if (!SetProperty(isolate->GetCurrentContext(), target, v8_name, function)) {
- LOG(WARNING) << "Could not bind log method \"" << name << "\"";
- }
- SetProperty(isolate->GetCurrentContext(), target, v8_name,
- tmpl->GetFunction());
-}
+gin::WrapperInfo kWrapperInfo = {gin::kEmbedderNativeGin};
} // namespace
-void Debug(content::RenderFrame* render_frame, const std::string& message) {
- AddMessage(render_frame, content::CONSOLE_MESSAGE_LEVEL_VERBOSE, message);
-}
-
-void Log(content::RenderFrame* render_frame, const std::string& message) {
- AddMessage(render_frame, content::CONSOLE_MESSAGE_LEVEL_INFO, message);
-}
-
-void Warn(content::RenderFrame* render_frame, const std::string& message) {
- AddMessage(render_frame, content::CONSOLE_MESSAGE_LEVEL_WARNING, message);
-}
-
-void Error(content::RenderFrame* render_frame, const std::string& message) {
- AddMessage(render_frame, content::CONSOLE_MESSAGE_LEVEL_ERROR, message);
-}
-
void Fatal(content::RenderFrame* render_frame, const std::string& message) {
- Error(render_frame, message);
+ AddMessage(render_frame, content::CONSOLE_MESSAGE_LEVEL_ERROR, message);
CheckWithMinidump(message);
}
@@ -123,12 +83,30 @@ void AddMessage(content::RenderFrame* render_frame,
v8::Local<v8::Object> AsV8Object(v8::Isolate* isolate) {
v8::EscapableHandleScope handle_scope(isolate);
- v8::Local<v8::Object> console_object = v8::Object::New(isolate);
- BindLogMethod(isolate, console_object, "debug", &Debug);
- BindLogMethod(isolate, console_object, "log", &Log);
- BindLogMethod(isolate, console_object, "warn", &Warn);
- BindLogMethod(isolate, console_object, "error", &Error);
- return handle_scope.Escape(console_object);
+ gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
+ v8::Local<v8::ObjectTemplate> templ = data->GetObjectTemplate(&kWrapperInfo);
+ if (templ.IsEmpty()) {
+ templ = v8::ObjectTemplate::New(isolate);
+ static const struct {
+ const char* name;
+ content::ConsoleMessageLevel level;
+ } methods[] = {
+ {"debug", content::CONSOLE_MESSAGE_LEVEL_VERBOSE},
+ {"log", content::CONSOLE_MESSAGE_LEVEL_INFO},
+ {"warn", content::CONSOLE_MESSAGE_LEVEL_WARNING},
+ {"error", content::CONSOLE_MESSAGE_LEVEL_ERROR},
+ };
+ for (const auto& method : methods) {
+ v8::Local<v8::FunctionTemplate> function =
+ v8::FunctionTemplate::New(isolate, BoundLogMethodCallback,
+ v8::Integer::New(isolate, method.level));
+ function->RemovePrototype();
+ templ->Set(gin::StringToSymbol(isolate, method.name), function);
+ }
+ data->SetObjectTemplate(&kWrapperInfo, templ);
+ }
+ return handle_scope.Escape(
+ templ->NewInstance(isolate->GetCurrentContext()).ToLocalChecked());
}
} // namespace console
« no previous file with comments | « extensions/renderer/console.h ('k') | extensions/renderer/module_system.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698