Issue 2325 - Add a way to set settings in libadblockplus for FRP and automatic updates

src/JsEngine.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 52 matching lines @@
       static V8Initializer initializer;
     }
   };
 }
 
 AdblockPlus::JsEngine::JsEngine()
   : isolate(v8::Isolate::GetCurrent())
 {
 }
 
 AdblockPlus::JsEnginePtr AdblockPlus::JsEngine::New(const AppInfo& appInfo)

[Eric, 2015/06/10 17:48:16]

This is perhaps beyond the scope of this change set, but there doesn't seem to
be anything here that couldn't be done in the 'JsEngine' constructor. There
seems to be no need for a factory method.

I mention this principally to point out that this is the _de facto_ constructor
for 'JsEngine', and this is salient to the initialization of 'globalJsObject'.

[Felix Dahlke, 2015/06/11 20:21:13]

I frankly don't recall why we did it that way, possibly because the v8
initialisation has to happen before the members of JsEngine are initialised. But
yeah, out of scope here.

 {
   V8Initializer::Init();
   JsEnginePtr result(new JsEngine());
 
   const v8::Locker locker(result->isolate);
   const v8::HandleScope handleScope;
 
   result->context.reset(result->isolate, v8::Context::New(result->isolate));
-  v8::Local<v8::Object> globalContext = v8::Local<v8::Context>::New(
-    result->isolate, result->context)->Global();
-  AdblockPlus::GlobalJsObject::Setup(result, appInfo,
-    JsValuePtr(new JsValue(result, globalContext)));
+  AdblockPlus::GlobalJsObject::Setup(result, appInfo, 
+    result->GetGlobalJsObject());
   return result;
 }
 
 AdblockPlus::JsValuePtr AdblockPlus::JsEngine::Evaluate(const std::string& source,
     const std::string& filename)
 {
   const JsContext context(shared_from_this());
   const v8::TryCatch tryCatch;
   const v8::Handle<v8::Script> script = CompileScript(isolate, source,
     filename);
@@ skipping 124 matching lines @@
   return logSystem;
 }
 
 void AdblockPlus::JsEngine::SetLogSystem(AdblockPlus::LogSystemPtr val)
 {
   if (!val)
     throw std::runtime_error("LogSystem cannot be null");
 
   logSystem = val;
 }
+
+AdblockPlus::JsValuePtr AdblockPlus::JsEngine::GetGlobalJsObject()
+{
+  if (!globalJsObject)

[Eric, 2015/06/10 17:48:16]

There's really no need to use deferred evaluation for this. 

'globalJsObject' is a member constant, as it's initialized in the (_de facto_)
constructor and never modified thereafter. (Reason: 'SetGlobalJsObject' is never
called.) 'globalJsOjbect' should be initialized in the constructor, or, barring
that, in the factory method 'New'.

This function should be a simple 'const' accessor.

+  {
+    v8::Local<v8::Object> globalContext = v8::Local<v8::Context>::New(
+      isolate, context)->Global();
+    globalJsObject = JsValuePtr(
+      new JsValue(JsEnginePtr(shared_from_this()), globalContext));
+  }
+  return globalJsObject;
+}
+
+void AdblockPlus::JsEngine::SetGlobalJsObject(AdblockPlus::JsValuePtr global)
+{
+  if (!global)
+    throw std::runtime_error("Global object cannot be null");
+  globalJsObject = global;
+}