Rewrite extensions console to not leak function templates.

extensions/renderer/console.cc

Index: extensions/renderer/console.cc
diff --git a/extensions/renderer/console.cc b/extensions/renderer/console.cc
index 30e42eb34b56ac5ac7c34a1ae12d1c4710b2fcae..4cca13e59d6914d713b0877652ab69971623ee2a 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,66 +44,26 @@ 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);
+  content::RenderFrame* render_frame =
+      script_context ? script_context->GetRenderFrame() : nullptr;

[Devlin, 2017/02/27 22:32:12]

I think we *should* be able to (D)CHECK() script_context here, but we don't have
to do that in this CL (in case for some reason we can't).  Maybe add a TODO?

[jbroman, 2017/02/27 22:52:49]

Added a TODO(devlin). I'm not familiar enough with ScriptContextSet off the top
of my head.

+  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);
   CheckWithMinidump(message);
@@ -123,12 +82,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