Simplify context setup, set properties on the global object directly instead of using templates

src/GlobalJsObject.cpp

 #include <vector>
 #include <stdexcept>
 
 #include <AdblockPlus/JsEngine.h>
 #include <AdblockPlus/JsValue.h>
 
 #include "AppInfoJsObject.h"
 #include "ConsoleJsObject.h"
 #include "FileSystemJsObject.h"
 #include "GlobalJsObject.h"
@@ skipping 52 matching lines @@
     }
     timeoutThread->Start();
 
     // We should actually return the timer ID here, which could be
     // used via clearTimeout(). But since we don't seem to need
     // clearTimeout(), we can save that for later.
     return v8::Undefined();
   }
 }
 
-v8::Handle<v8::ObjectTemplate> GlobalJsObject::Create(
-  const AppInfo& appInfo, JsEngine& jsEngine)
+JsValuePtr GlobalJsObject::Setup(JsEngine& jsEngine, const AppInfo& appInfo,
+    JsValuePtr obj)
 {
-  const v8::Locker locker(v8::Isolate::GetCurrent());
-  v8::HandleScope handleScope;
-  const v8::Handle<v8::ObjectTemplate> global = v8::ObjectTemplate::New();
-  const v8::Handle<v8::FunctionTemplate> setTimeoutFunction =
-    v8::FunctionTemplate::New(SetTimeoutCallback,
-                              v8::External::New(&jsEngine));
-  global->Set(v8::String::New("setTimeout"), setTimeoutFunction);
-  const v8::Handle<v8::ObjectTemplate> fileSystemObject =
-    FileSystemJsObject::Create(jsEngine);
-  global->Set(v8::String::New("_fileSystem"), fileSystemObject);
-  const v8::Handle<v8::ObjectTemplate> webRequestObject =
-    WebRequestJsObject::Create(jsEngine);
-  global->Set(v8::String::New("_webRequest"), webRequestObject);
-  const v8::Handle<v8::ObjectTemplate> consoleObject =
-    ConsoleJsObject::Create(jsEngine);
-  global->Set(v8::String::New("console"), consoleObject);
-  const v8::Handle<v8::ObjectTemplate> appInfoObject =
-    AppInfoJsObject::Create(appInfo);
-  global->Set(v8::String::New("_appInfo"), appInfoObject);
-  return handleScope.Close(global);
+  obj->SetProperty("setTimeout", jsEngine.NewCallback(::SetTimeoutCallback));
+  obj->SetProperty("_fileSystem",
+      FileSystemJsObject::Setup(jsEngine, jsEngine.NewObject()));

[Felix Dahlke, 2013/04/18 12:46:50]

Why pass a new object in? Seems to make more sense for the setup functions to
create their own object and return it.

[Wladimir Palant, 2013/04/18 13:29:33]

Merely for consistency. GlobaJsObject::Setup() cannot create a new object, it
has to work with the existing global. I can change it for all the other Setup()
functions if you prefer but then they will work differently from
GlobalJsObject::Setup().

+  obj->SetProperty("_webRequest",
+      WebRequestJsObject::Setup(jsEngine, jsEngine.NewObject()));
+  obj->SetProperty("console",
+      ConsoleJsObject::Setup(jsEngine, jsEngine.NewObject()));
+  obj->SetProperty("_appInfo",
+      AppInfoJsObject::Setup(jsEngine, appInfo, jsEngine.NewObject()));
+  return obj;
 }