Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: include/AdblockPlus/JsEngine.h

Issue 29371607: Issue #3593 - Make isolate a fully internal member of the engine
Patch Set: improve unit tests to go with isolate change Created Jan. 16, 2017, 3:53 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | src/JsEngine.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: include/AdblockPlus/JsEngine.h
===================================================================
--- a/include/AdblockPlus/JsEngine.h
+++ b/include/AdblockPlus/JsEngine.h
@@ -50,31 +50,6 @@
typedef std::shared_ptr<JsEngine> JsEnginePtr;
/**
- * Scope based isolate manager. Creates a new isolate instance on
- * constructing and disposes it on destructing.
- */
- class ScopedV8Isolate
- {
- public:
- ScopedV8Isolate();
- ~ScopedV8Isolate();
- v8::Isolate* Get()
- {
- return isolate;
- }
- private:
- ScopedV8Isolate(const ScopedV8Isolate&);
- ScopedV8Isolate& operator=(const ScopedV8Isolate&);
-
- v8::Isolate* isolate;
- };
-
- /**
- * Shared smart pointer to ScopedV8Isolate instance;
- */
- typedef std::shared_ptr<ScopedV8Isolate> ScopedV8IsolatePtr;
-
- /**
* JavaScript engine used by `FilterEngine`, wraps v8.
*/
class JsEngine : public std::enable_shared_from_this<JsEngine>
@@ -97,7 +72,7 @@
* as a temporary hack for tests, it will go away. Issue #3593.
* @return New `JsEngine` instance.
*/
- static JsEnginePtr New(const AppInfo& appInfo = AppInfo(), const ScopedV8IsolatePtr& isolate = ScopedV8IsolatePtr(new ScopedV8Isolate()));
+ static JsEnginePtr New(const AppInfo& appInfo = AppInfo());
/**
* Registers the callback function for an event.
@@ -243,13 +218,22 @@
/**
* Returns a pointer to associated v8::Isolate.
*/
- v8::Isolate* GetIsolate()
+ v8::Isolate* GetIsolate() const
{
- return isolate->Get();
+ return isolate;
}
+ /// Ordinary destructor
+ ~JsEngine();
+
protected:
- explicit JsEngine(const ScopedV8IsolatePtr& isolate);
+ /**
+ * Non-public constructor enforces factory use.
+ *
+ * @param isolate
+ * v8 isolate obtained from its factory
+ */
+ explicit JsEngine(v8::Isolate* isolate);
/**
* Retrieve the global object as a JsValuePtr
@@ -259,9 +243,12 @@
*/
JsValuePtr GetGlobalObject();
- /// Isolate must be disposed only after disposing of all objects which are
- /// using it.
- ScopedV8IsolatePtr isolate;
+ /**
+ * Isolate member must be have life span that encompasses any other object
+ * that uses it. Thus it's declared first so that it is constructed first
+ * and destroyed last.
+ */
+ v8::Isolate* isolate;
FileSystemPtr fileSystem;
WebRequestPtr webRequest;
« no previous file with comments | « no previous file | src/JsEngine.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld