Extend DOMActivityLogger interface for upcoming change

In order to reduce pain (hopefully to zero) for sheriffs and fellow developers
who prefer their trees green rather than red, let's make landing the change
at https://codereview.chromium.org/213783002/ as safe as possible.

Step 1: This CL. Extend DOMActivityLogger with new methods for logGetter,
logSetter, and logMethod, which have the same functionality as the current
log() method.
Step 2: https://codereview.chromium.org/213783002/ - The implementation change
in blink to switch from using log() to using logX.
Step 3: Cleanup -- remove old log method from chrome.

BUG=356890

Review URL: https://codereview.chromium.org/247953008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266157 0039d316-1c4b-4281-b951-d872f2087c98

chrome/common/extensions/ad_injection_constants.cc

@@ -29,7 +29,7 @@ extern const size_t kMaximumChildrenToCheck = 10u;
 // The maximum depth to check when we examine a newly-added element.
 extern const size_t kMaximumDepthToCheck = 5u;
 
-bool ApiCanInjectAds(const char* api) {
+bool ApiCanInjectAds(const std::string& api) {
   return api == kHtmlIframeSrcApiName ||
          api == kHtmlEmbedSrcApiName ||
          EndsWith(api, kAppendChildApiSuffix, true /* case sensitive */);

chrome/common/extensions/ad_injection_constants.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_COMMON_EXTENSIONS_AD_INJECTION_CONSTANTS_H_
 #define CHROME_COMMON_EXTENSIONS_AD_INJECTION_CONSTANTS_H_
 
+#include <string>
+
 #include "base/basictypes.h"
 
 namespace extensions {
@@ -29,7 +31,7 @@ extern const size_t kMaximumDepthToCheck;
 
 // Returns true if the given |api| can potentially inject ads, and should
 // therefore be examined.
-bool ApiCanInjectAds(const char* api);
+bool ApiCanInjectAds(const std::string& api);
 
 }  // namespace ad_injection_constants
 }  // namespace extensions

chrome/renderer/extensions/dom_activity_logger.cc

@@ -5,7 +5,6 @@
 #include "chrome/renderer/extensions/dom_activity_logger.h"
 
 #include "chrome/common/extensions/ad_injection_constants.h"
-#include "chrome/common/extensions/dom_action_types.h"
 #include "chrome/renderer/chrome_render_process_observer.h"
 #include "chrome/renderer/extensions/activity_log_converter_strategy.h"
 #include "content/public/renderer/render_thread.h"
@@ -22,6 +21,22 @@ using blink::WebURL;
 
 namespace extensions {
 
+namespace {
+
+scoped_ptr<base::Value> ConvertV8Value(const std::string& api_name,
+                                       const v8::Handle<v8::Value> v8_value) {
+  scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
+  ActivityLogConverterStrategy strategy;
+  strategy.set_enable_detailed_parsing(
+      ad_injection_constants::ApiCanInjectAds(api_name));
+  converter->SetFunctionAllowed(true);
+  converter->SetStrategy(&strategy);
+  return scoped_ptr<base::Value>(converter->FromV8Value(
+      v8_value, v8::Isolate::GetCurrent()->GetCurrentContext()));
+}
+
+}  // namespace
+
 DOMActivityLogger::DOMActivityLogger(const std::string& extension_id)
     : extension_id_(extension_id) {
 }
@@ -35,35 +50,19 @@ void DOMActivityLogger::log(
     const WebString& call_type,
     const WebURL& url,
     const WebString& title) {
-  scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
-  ActivityLogConverterStrategy strategy;
-  strategy.set_enable_detailed_parsing(
-      ad_injection_constants::ApiCanInjectAds(api_name.utf8().c_str()));
-  converter->SetFunctionAllowed(true);
-  converter->SetStrategy(&strategy);
-  scoped_ptr<base::ListValue> argv_list_value(new base::ListValue());
-  for (int i = 0; i < argc; i++) {
-    argv_list_value->Set(
-        i,
-        converter->FromV8Value(argv[i],
-                               v8::Isolate::GetCurrent()->GetCurrentContext()));
-  }
-
-  ExtensionHostMsg_DOMAction_Params params;
-  params.url = url;
-  params.url_title = title;
-  params.api_call = api_name.utf8();
-  params.arguments.Swap(argv_list_value.get());
-  const std::string type = call_type.utf8();
-  if (type == "Getter")
-    params.call_type = DomActionType::GETTER;
-  else if (type == "Setter")
-    params.call_type = DomActionType::SETTER;
-  else
-    params.call_type = DomActionType::METHOD;
+  scoped_ptr<base::ListValue> args(new base::ListValue());
+  std::string api_name_utf8 = api_name.utf8();
+  for (int i = 0; i < argc; ++i)
+    args->Append(ConvertV8Value(api_name_utf8, argv[i]).release());
 
-  content::RenderThread::Get()->Send(
-      new ExtensionHostMsg_AddDOMActionToActivityLog(extension_id_, params));
+  DomActionType::Type type = DomActionType::METHOD;
+  if (call_type == "Getter")
+    type = DomActionType::GETTER;
+  else if (call_type == "Setter")
+    type = DomActionType::SETTER;
+  // else DomActionType::METHOD is correct.
+  SendDomActionMessage(
+      api_name_utf8, url, title, type, args.Pass());
 }
 
 void DOMActivityLogger::AttachToWorld(int world_id,
@@ -78,4 +77,63 @@ void DOMActivityLogger::AttachToWorld(int world_id,
 #endif
 }
 
+void DOMActivityLogger::logGetter(const WebString& api_name,
+                                  const WebURL& url,
+                                  const WebString& title) {
+  SendDomActionMessage(api_name.utf8(),
+                       url,
+                       title,
+                       DomActionType::GETTER,
+                       scoped_ptr<base::ListValue>(new base::ListValue()));
+}
+
+void DOMActivityLogger::logSetter(const WebString& api_name,
+                                  const v8::Handle<v8::Value>& new_value,
+                                  const WebURL& url,
+                                  const WebString& title) {
+  logSetter(api_name, new_value, v8::Handle<v8::Value>(), url, title);
+}
+
+void DOMActivityLogger::logSetter(const WebString& api_name,
+                                  const v8::Handle<v8::Value>& new_value,
+                                  const v8::Handle<v8::Value>& old_value,
+                                  const WebURL& url,
+                                  const WebString& title) {
+  scoped_ptr<base::ListValue> args(new base::ListValue);
+  std::string api_name_utf8 = api_name.utf8();
+  args->Append(ConvertV8Value(api_name_utf8, new_value).release());
+  if (!old_value.IsEmpty())
+    args->Append(ConvertV8Value(api_name_utf8, old_value).release());
+  SendDomActionMessage(
+      api_name_utf8, url, title, DomActionType::SETTER, args.Pass());
+}
+
+void DOMActivityLogger::logMethod(const WebString& api_name,
+                                  int argc,
+                                  const v8::Handle<v8::Value>* argv,
+                                  const WebURL& url,
+                                  const WebString& title) {
+  scoped_ptr<base::ListValue> args(new base::ListValue);
+  std::string api_name_utf8 = api_name.utf8();
+  for (int i = 0; i < argc; ++i)
+    args->Append(ConvertV8Value(api_name_utf8, argv[i]).release());
+  SendDomActionMessage(
+      api_name_utf8, url, title, DomActionType::METHOD, args.Pass());
+}
+
+void DOMActivityLogger::SendDomActionMessage(const std::string& api_call,
+                                             const GURL& url,
+                                             const base::string16& url_title,
+                                             DomActionType::Type call_type,
+                                             scoped_ptr<base::ListValue> args) {
+  ExtensionHostMsg_DOMAction_Params params;
+  params.api_call = api_call;
+  params.url = url;
+  params.url_title = url_title;
+  params.call_type = call_type;
+  params.arguments.Swap(args.get());
+  content::RenderThread::Get()->Send(
+      new ExtensionHostMsg_AddDOMActionToActivityLog(extension_id_, params));
+}
+
 }  // namespace extensions

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,
+                         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,
+                         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);