Extend DOMActivityLogger interface for upcoming change

chrome/renderer/extensions/dom_activity_logger.h

Index: chrome/renderer/extensions/dom_activity_logger.h
diff --git a/chrome/renderer/extensions/dom_activity_logger.h b/chrome/renderer/extensions/dom_activity_logger.h
index 19294f70f35c41f8e1d4122a9b51d581b2bfcb25..a37868fba532fb08237db46337a1d1d880ab95d0 100644
--- a/chrome/renderer/extensions/dom_activity_logger.h
+++ b/chrome/renderer/extensions/dom_activity_logger.h
@@ -8,20 +8,23 @@
 #include <string>
 
 #include "base/strings/string_piece.h"
+#include "chrome/common/extensions/dom_action_types.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 #include "third_party/WebKit/public/web/WebDOMActivityLogger.h"
 #include "url/gurl.h"
 #include "v8/include/v8.h"
 
+namespace base {
+class ListValue;
+}
+
 namespace content {
 class V8ValueConverter;
 }
 
 namespace extensions {
 
-class ActivityLogConverterStrategy;
-
 // Used to log DOM API calls from within WebKit. The events are sent via IPC to
 // extensions::ActivityLog for recording and display.
 class DOMActivityLogger: public blink::WebDOMActivityLogger {
@@ -30,16 +33,13 @@ class DOMActivityLogger: public blink::WebDOMActivityLogger {
   explicit DOMActivityLogger(const std::string& extension_id);
   virtual ~DOMActivityLogger();
 
-  // Marshalls the arguments into an ExtensionHostMsg_DOMAction_Params
-  // and sends it over to the browser (via IPC) for appending it to the
-  // extension activity log.
-  // (Overrides the log method in blink::WebDOMActivityLogger)
+  // This will soon be deprecated, and converted to the logX methods below.
   virtual void log(const blink::WebString& api_name,
                    int argc,
                    const v8::Handle<v8::Value> argv[],
                    const blink::WebString& call_type,
                    const blink::WebURL& url,
-                   const blink::WebString& title) OVERRIDE;
+                   const blink::WebString& title);
 
   // Check (using the WebKit API) if there is no logger attached to the world
   // corresponding to world_id, and if so, construct a new logger and attach it.
@@ -48,6 +48,38 @@ class DOMActivityLogger: public blink::WebDOMActivityLogger {
                             const std::string& extension_id);
 
  private:
+  // blink::WebDOMActivityLogger implementation.
+  // Marshals the arguments into an ExtensionHostMsg_DOMAction_Params and sends
+  // it over to the browser (via IPC) for appending it to the extension activity
+  // log.
+  // These methods don't have the OVERRIDE keyword due to the complexities it
+  // introduces when changes blink apis.
+  virtual void logGetter(const blink::WebString& api_name,
+                         const blink::WebURL& url,
+                         const blink::WebString& title);
+  virtual void logSetter(const blink::WebString& api_name,
+                         const v8::Handle<v8::Value>& new_value,

[not at google - send to devlin, 2014/04/24 17:35:56]

the only difference is that this doesn't take an old_value? can you just pass in
an empty v8::Handle in that case?

[Devlin, 2014/04/24 17:45:30]

Haha I did, originally, but blink folks preferred overloading the method. :)
https://codereview.chromium.org/213783002/diff/40001/Source/bindings/template...

Since it's a blink interface, I deferred to them.  And it is a little faster,
because all the empty values add up - we'd have to construct an empty V8 value,
and then convert it (which I think constructs an empty base::Value) and then
pass it over IPC, and then create add to the action, etc... gets a bit much,
IMO.

[not at google - send to devlin, 2014/04/24 17:48:38]

you don't need to construct an empty *value* just an empty *handle*.

then the implementation of logSetter would check for handle.IsEmpty().

[Devlin, 2014/04/24 17:57:55]

Ah, right.  Okay, done on chrome side.

+                         const blink::WebURL& url,
+                         const blink::WebString& title);
+  virtual void logSetter(const blink::WebString& api_name,
+                         const v8::Handle<v8::Value>& new_value,
+                         const v8::Handle<v8::Value>& old_value,

[not at google - send to devlin, 2014/04/24 17:35:56]

i find it odd taking the new value after the odd in the parameter list, but
maybe you don't, and I don't feel strongly

[Devlin, 2014/04/24 17:45:30]

{assuming you meant taking old after new}
Mainly, this is because we always get the new value, making the old value
"optional" -- this ensures that the new value is always at the same point in the
arg list (which is important since we serialize it).

+                         const blink::WebURL& url,
+                         const blink::WebString& title);
+  virtual void logMethod(const blink::WebString& api_name,
+                         int argc,
+                         const v8::Handle<v8::Value>* argv,
+                         const blink::WebURL& url,
+                         const blink::WebString& title);
+
+  // Helper function to actually send the message across IPC.
+  void SendDomActionMessage(const std::string& api_call,
+                            const GURL& url,
+                            const base::string16& url_title,
+                            DomActionType::Type call_type,
+                            scoped_ptr<base::ListValue> args);
+
+  // The id of the extension with which this logger is associated.
   std::string extension_id_;
 
   DISALLOW_COPY_AND_ASSIGN(DOMActivityLogger);