Issue 1280 - Update v8

include/AdblockPlus/JsValue.h

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 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 10 matching lines @@
 #include <stdint.h>
 #include <string>
 #include <vector>
 #include "tr1_memory.h"
 #include "V8ValueHolder.h"
 
 namespace v8
 {
   class Value;
   template<class T> class Handle;
+  template<class T> class Local;
 }
 
 namespace AdblockPlus
 {
   class JsValue;
   class JsEngine;
 
   typedef std::tr1::shared_ptr<JsEngine> JsEnginePtr;
 
   /**
    * Shared smart pointer to a `JsValue` instance.
    */
   typedef std::tr1::shared_ptr<JsValue> JsValuePtr;
 
   /**
    * List of JavaScript values.
    */
   typedef std::vector<AdblockPlus::JsValuePtr> JsValueList;
 
   /**
    * Wrapper for JavaScript values.
    * See `JsEngine` for creating `JsValue` objects.
    */
   class JsValue
   {
-    friend class JsEngine;
   public:
-    JsValue(JsValuePtr value);
+    struct Private
+    {
+    private:
+      // allow access to the ctr only JsEngine and don't allow JsEngine to access other members.
+      friend class JsEngine;
+      friend class JsValue;
+      struct CtrArg

[Felix Dahlke, 2014/10/31 05:31:57]

So the idea is that only friend classes can access this and thus only they can
create new JsValue objects? I'm not sure if I like it, but I would definitely
like to tackle this in a separate issue and focus on updating V8 and supporting
Visual Studio 2010 here.

[sergei, 2014/10/31 11:47:01]

Here it's merely another form of `friend class JsEngine;`, so it was here
already. But I feel I need to explain the trick used here.
Despite mentioned above the proper access to the members, the main requirement
to use it is such functions like std::make_shared
http://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared . 
Let's consider an example:
class NotUsableWithFactoryFunctions
{
  friend class Client;
protected: // support inheritance, otherwise private
  NotUsableWithFactoryFunctions(){}
};

class Client
{
  void SomeMethod()
  {
    std::make_unique<NotUsableWithFactoryFunctions>(); // compilation error
because `std::make_unique` does not have access to the constructor of
`NotUsableWithFactoryFunctions`.
  }
};

Adding such functions as friends of `NotUsableWithFactoryFunctions` makes no
sense because in that case everybody can instantiate them.
The following class allows `Client` to construct an instance of `ClientFriendly`
using `std::make_unique<ClientFriendly>(ClientFriendly::PrivateArg{});`
class ClientFriendly
{
  friend class Client;
protected: // support inheritance, otherwise private
  struct PrivateArg{};
public:
  explicit NotUsableWithFactoryFunctions(PrivateArg){}
};

BTW, the same trick sometimes is used when one need to expose a pointer to the
member but does not want the member to be callable from outside.
class Button
{
protected:
  struct PrivateSignalArg{};
public:
  // signals
  void pushed(PrivateSignalArg);
};

Button button;
//button.pushed(); // cannot be called by anybody except `Button` and its
derived.
SomeModel model;
model.connect(&Button::pushed, &model, &SomeModel::onButtonPushed); // OK

May be the most famous example of it is in Qt5
http://woboq.com/blog/how-qt-signals-slots-work-part2-qt5.html # Protected,
Public, or Private Signals.


JIC, from http://en.cppreference.com/w/cpp/memory/shared_ptr/make_shared
This function is typically used to replace the construction
std::shared_ptr<T>(new T(args...)) of a shared pointer from the raw pointer
returned by a call to new. In contrast to that expression, std::make_shared<T>
typically allocates memory for the T object and for the std::shared_ptr's
control block with a single memory allocation (this is a non-binding requirement
in the Standard), where std::shared_ptr<T>(new T(args...)) performs at least two
memory allocations.
Moreover, code such as f(std::shared_ptr<int>(new int(42)), g()) can cause a
memory leak if g throws an exception because g() may be called after new int(42)
and before the constructor of shared_ptr<int>. This doesn't occur in
f(std::make_shared<int>(42), g()), since two function calls are never
interleaved.

Anyway it's just a good practice to use make_shared and make_unique instead of
manual calls `new`. But it should not be blind, http://herbsutter.com/gotw/_102/
.

[Felix Dahlke, 2014/10/31 15:07:51]

At first glance it seems like a rather complex hack around a missing language
feature to me. But I'd like to dig into this (and how Qt5 is using it) some more
before discussing it.

Anyway, we really should handle this in a dedicated issue/review.


I think I see the point, but I still think we shouldn't mix this up with issue
1280.

[sergei, 2014/10/31 16:13:35]

reverted.
JIC, it's mainly for std::make_{shared,unique}

+      {
+      };
+    };
+  protected:
+    typedef Private::CtrArg PrivateCtrArg;
+    JsValue(const JsValuePtr& value);
+  public:
+    JsValue(const JsEnginePtr& jsEngine, const v8::Handle<v8::Value> value, PrivateCtrArg);
     virtual ~JsValue();
 
     bool IsUndefined() const;
     bool IsNull() const;
     bool IsString() const;
     bool IsNumber() const;
     bool IsBool() const;
     bool IsObject() const;
     bool IsArray() const;
     bool IsFunction() const;
@@ skipping 18 matching lines @@
 
     //@{
     /**
      * Sets a property value if this is an object (see `IsObject()`).
      * @param name Property name.
      * @param val Property value.
      */
     void SetProperty(const std::string& name, const std::string& val);
     void SetProperty(const std::string& name, int64_t val);
     void SetProperty(const std::string& name, bool val);
-    void SetProperty(const std::string& name, JsValuePtr value);
+    void SetProperty(const std::string& name, const JsValuePtr& value);
     inline void SetProperty(const std::string& name, const char* val)
     {
       SetProperty(name, std::string(val));
     }
     inline void SetProperty(const std::string& name, int val)
     {
       SetProperty(name, static_cast<int64_t>(val));
     }
     //@}
 
     /**
      * Returns the value's class name, e.g.\ _Array_ for arrays
      * (see `IsArray()`).
      * Technically, this is the name of the function that was used as a
      * constructor.
      * @return Class name of the value.
      */
     std::string GetClass() const;
 
     /**
      * Invokes the value as a function (see `IsFunction()`).
      * @param params Optional list of parameters.
      * @param thisPtr Optional `this` value.
      * @return Value returned by the function.
      */
     JsValuePtr Call(const JsValueList& params = JsValueList(),
         AdblockPlus::JsValuePtr thisPtr = AdblockPlus::JsValuePtr()) const;
 
+    v8::Local<v8::Value> unwrapValue() const;

[Felix Dahlke, 2014/10/31 05:31:57]

All functions should start with an upper case letter.

[sergei, 2014/10/31 11:47:01]

fixed

+  private:
+    void SetProperty(const std::string& name, const v8::Handle<v8::Value> val);
   protected:
-    JsValue(JsEnginePtr jsEngine, v8::Handle<v8::Value> value);
-    void SetProperty(const std::string& name, v8::Handle<v8::Value> val);
-
     JsEnginePtr jsEngine;
+  private:
     V8ValueHolder<v8::Value> value;
   };
 }
 
 #endif