Added activity logging for ext APIs with custom bindings

The Activity Log wasn't catching chrome.* API calls if they went through custom bindings. This CL fixes that and adds a test case.

BUG=39802


Review URL: https://chromiumcodereview.appspot.com/12517011

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

chrome/browser/renderer_host/chrome_render_message_filter.cc

@@ -57,6 +57,31 @@ using WebKit::WebSecurityOrigin;
 
 namespace {
 
+void AddAPIActionToExtensionActivityLog(
+    Profile* profile,
+    const extensions::Extension* extension,
+    const std::string& api_call,
+    scoped_ptr<ListValue> args,
+    const std::string& extra) {
+  // The ActivityLog can only be accessed from the main (UI) thread.  If we're
+  // running on the wrong thread, re-dispatch from the main thread.
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(BrowserThread::UI,
+                            FROM_HERE,
+                            base::Bind(&AddAPIActionToExtensionActivityLog,
+                                       profile,
+                                       extension,
+                                       api_call,
+                                       base::Passed(&args),
+                                       extra));
+  } else {
+    extensions::ActivityLog* activity_log =
+        extensions::ActivityLog::GetInstance(profile);
+    if (activity_log && activity_log->IsLogEnabled())
+      activity_log->LogAPIAction(extension, api_call, args.get(), extra);
+  }
+}
+
 void AddDOMActionToExtensionActivityLog(
     Profile* profile,
     const extensions::Extension* extension,
@@ -81,7 +106,7 @@ void AddDOMActionToExtensionActivityLog(
   } else {
     extensions::ActivityLog* activity_log =
         extensions::ActivityLog::GetInstance(profile);
-    if (activity_log)
+    if (activity_log && activity_log->IsLogEnabled())
       activity_log->LogDOMAction(extension, url, url_title,
                                  api_call, args.get(), extra);
   }
@@ -153,6 +178,8 @@ bool ChromeRenderMessageFilter::OnMessageReceived(const IPC::Message& message,
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_SuspendAck, OnExtensionSuspendAck)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_ResumeRequests,
                         OnExtensionResumeRequests);
+    IPC_MESSAGE_HANDLER(ExtensionHostMsg_AddAPIActionToActivityLog,
+                        OnAddAPIActionToExtensionActivityLog);
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_AddDOMActionToActivityLog,
                         OnAddDOMActionToExtensionActivityLog);
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_AllowDatabase, OnAllowDatabase)
@@ -562,6 +589,19 @@ void ChromeRenderMessageFilter::OnExtensionResumeRequests(int route_id) {
       render_process_id_, route_id);
 }
 
+void ChromeRenderMessageFilter::OnAddAPIActionToExtensionActivityLog(
+    const std::string& extension_id,
+    const ExtensionHostMsg_APIAction_Params& params) {
+  const extensions::Extension* extension =
+      extension_info_map_->extensions().GetByID(extension_id);
+  scoped_ptr<ListValue> args(params.arguments.DeepCopy());
+  // The activity is recorded as an API action in the extension
+  // activity log.
+  AddAPIActionToExtensionActivityLog(profile_, extension,
+                                     params.api_call, args.Pass(),
+                                     params.extra);
+}
+
 void ChromeRenderMessageFilter::OnAddDOMActionToExtensionActivityLog(
     const std::string& extension_id,
     const ExtensionHostMsg_DOMAction_Params& params) {

chrome/browser/renderer_host/chrome_render_message_filter.h

@@ -17,6 +17,7 @@
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebCache.h"
 
 class CookieSettings;
+struct ExtensionHostMsg_APIAction_Params;
 struct ExtensionHostMsg_DOMAction_Params;
 struct ExtensionHostMsg_Request_Params;
 class ExtensionInfoMap;
@@ -152,6 +153,9 @@ class ChromeRenderMessageFilter : public content::BrowserMessageFilter {
   void OnExtensionSuspendAck(const std::string& extension_id);
   void OnExtensionGenerateUniqueID(int* unique_id);
   void OnExtensionResumeRequests(int route_id);
+  void OnAddAPIActionToExtensionActivityLog(
+      const std::string& extension_id,
+      const ExtensionHostMsg_APIAction_Params& params);
   void OnAddDOMActionToExtensionActivityLog(
       const std::string& extension_id,
       const ExtensionHostMsg_DOMAction_Params& params);

chrome/chrome_renderer.gypi

@@ -48,6 +48,8 @@
         'renderer/automation/automation_renderer_helper.h',
         'renderer/benchmarking_extension.cc',
         'renderer/benchmarking_extension.h',
+        'renderer/extensions/api_activity_logger.cc',
+        'renderer/extensions/api_activity_logger.h',
         'renderer/extensions/api_definitions_natives.cc',
         'renderer/extensions/api_definitions_natives.h',
         'renderer/extensions/app_bindings.cc',

chrome/common/extensions/extension_messages.h

@@ -30,6 +30,18 @@
 
 IPC_ENUM_TRAITS(chrome::ViewType)
 
+// Parameters structure for ExtensionHostMsg_AddAPIActionToActivityLog.
+IPC_STRUCT_BEGIN(ExtensionHostMsg_APIAction_Params)
+  // API name.
+  IPC_STRUCT_MEMBER(std::string, api_call)
+
+  // List of arguments.
+  IPC_STRUCT_MEMBER(ListValue, arguments)
+
+  // Extra logging information.
+  IPC_STRUCT_MEMBER(std::string, extra)
+IPC_STRUCT_END()
+
 // Parameters structure for ExtensionHostMsg_AddDOMActionToActivityLog.
 IPC_STRUCT_BEGIN(ExtensionHostMsg_DOMAction_Params)
   // URL of the page.
@@ -588,7 +600,12 @@ IPC_MESSAGE_CONTROL1(ExtensionHostMsg_ResumeRequests, int /* route_id */)
 IPC_MESSAGE_ROUTED1(ExtensionHostMsg_UpdateDraggableRegions,
                     std::vector<extensions::DraggableRegion> /* regions */)
 
-// Sent by the renderer to log a DOM action on the extension activity log.
+// Sent by the renderer to log an API action to the extension activity log.
+IPC_MESSAGE_CONTROL2(ExtensionHostMsg_AddAPIActionToActivityLog,
+                     std::string /* extension_id */,
+                     ExtensionHostMsg_APIAction_Params)
+
+// Sent by the renderer to log a DOM action to the extension activity log.
 IPC_MESSAGE_CONTROL2(ExtensionHostMsg_AddDOMActionToActivityLog,
                      std::string /* extension_id */,
                      ExtensionHostMsg_DOMAction_Params)

chrome/renderer/extensions/api_activity_logger.cc

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <string>
+#include "base/bind.h"
+#include "chrome/common/extensions/extension_messages.h"
+#include "chrome/renderer/chrome_render_process_observer.h"
+#include "chrome/renderer/extensions/api_activity_logger.h"
+#include "content/public/renderer/render_thread.h"
+#include "content/public/renderer/v8_value_converter.h"
+
+using content::V8ValueConverter;
+
+namespace extensions {
+
+APIActivityLogger::APIActivityLogger(
+    Dispatcher* dispatcher, v8::Handle<v8::Context> v8_context)
+    : ChromeV8Extension(dispatcher, v8_context) {
+  RouteFunction("LogActivity", base::Bind(&APIActivityLogger::LogActivity));
+}
+
+// static
+v8::Handle<v8::Value> APIActivityLogger::LogActivity(
+    const v8::Arguments& args) {
+  DCHECK_GT(args.Length(), 2);
+  DCHECK(args[0]->IsString());
+  DCHECK(args[1]->IsString());
+  DCHECK(args[2]->IsArray());
+
+  // Get the simple values.
+  std::string ext_id = *v8::String::AsciiValue(args[0]->ToString());
+  ExtensionHostMsg_APIAction_Params params;
+  params.api_call = *v8::String::AsciiValue(args[1]->ToString());
+  if (args.Length() == 4)  // Extras are optional.
+    params.extra = *v8::String::AsciiValue(args[3]->ToString());
+  else
+    params.extra = "";
+
+  // Get the array of api call arguments.
+  v8::Local<v8::Array> arg_array = v8::Local<v8::Array>::Cast(args[2]);
+  if (arg_array->Length() > 0) {
+    scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
+    scoped_ptr<ListValue> arg_list(new ListValue());
+    for (size_t i = 0; i < arg_array->Length(); ++i) {
+      arg_list->Set(i,
+                    converter->FromV8Value(arg_array->Get(i),
+                    v8::Context::GetCurrent()));
+    }
+    params.arguments.Swap(arg_list.get());
+  }
+
+  content::RenderThread::Get()->Send(
+      new ExtensionHostMsg_AddAPIActionToActivityLog(ext_id, params));
+
+  return v8::Undefined();
+}
+
+}  // namespace extensions
+

chrome/renderer/extensions/api_activity_logger.h

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_RENDERER_EXTENSIONS_API_ACTIVITY_LOGGER_H_
+#define CHROME_RENDERER_EXTENSIONS_API_ACTIVITY_LOGGER_H_
+
+#include <string>
+#include "chrome/renderer/extensions/chrome_v8_extension.h"
+#include "chrome/renderer/extensions/dispatcher.h"
+#include "v8/include/v8.h"
+
+namespace extensions {
+
+// Used to log extension API calls that are implemented with custom bindings.
+// The events are sent via IPC to extensions::ActivityLog for recording and
+// display.
+class APIActivityLogger : public ChromeV8Extension {
+ public:
+  APIActivityLogger(Dispatcher* dispatcher, v8::Handle<v8::Context> v8_context);
+
+  // This is ultimately invoked in schema_generated_bindings.js with
+  // JavaScript arguments...
+  //    arg0 - extension ID as a string
+  //    arg1 - API call name as a string
+  //    arg2 - arguments to the API call
+  //    arg3 - any extra logging info as a string (optional)
+  static v8::Handle<v8::Value> LogActivity(const v8::Arguments & args);
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(APIActivityLogger);
+};
+
+}  // namespace extensions
+
+#endif  // CHROME_RENDERER_EXTENSIONS_API_ACTIVITY_LOGGER_H_
+

chrome/renderer/extensions/dispatcher.cc

@@ -23,6 +23,7 @@
 #include "chrome/common/url_constants.h"
 #include "chrome/common/view_type.h"
 #include "chrome/renderer/chrome_render_process_observer.h"
+#include "chrome/renderer/extensions/api_activity_logger.h"
 #include "chrome/renderer/extensions/api_definitions_natives.h"
 #include "chrome/renderer/extensions/app_bindings.h"
 #include "chrome/renderer/extensions/app_runtime_custom_bindings.h"
@@ -728,6 +729,8 @@ void Dispatcher::RegisterNativeHandlers(ModuleSystem* module_system,
   module_system->RegisterNativeHandler(
       "contentWatcherNative",
       content_watcher_->MakeNatives(v8_context));
+  module_system->RegisterNativeHandler("activityLogger",
+      scoped_ptr<NativeHandler>(new APIActivityLogger(this, v8_context)));
 
   // Natives used by multiple APIs.
   module_system->RegisterNativeHandler("file_system_natives",

chrome/renderer/extensions/dom_activity_logger.h

@@ -15,6 +15,8 @@
 
 using WebKit::WebString;
 
+// Used to log DOM API calls from within WebKit. The events are sent via IPC to
+// extensions::ActivityLog for recording and display.
 namespace extensions {
 
 class DOMActivityLogger: public WebKit::WebDOMActivityLogger {

chrome/renderer/resources/extensions/binding.js

@@ -15,16 +15,19 @@ var extensionId = process.GetExtensionId();
 var manifestVersion = process.GetManifestVersion();
 var schemaRegistry = requireNative('schema_registry');
 var schemaUtils = require('schemaUtils');
-var sendRequest = require('sendRequest').sendRequest;
 var utils = require('utils');
 var CHECK = requireNative('logging').CHECK;
+var sendRequestHandler = require('sendRequest');
+var sendRequest = sendRequestHandler.sendRequest;
+var logActivity = requireNative('activityLogger').LogActivity;
 
 // Stores the name and definition of each API function, with methods to
 // modify their behaviour (such as a custom way to handle requests to the
 // API, a custom callback, etc).
-function APIFunctions() {
+function APIFunctions(namespace) {
   this.apiFunctions_ = {};
   this.unavailableApiFunctions_ = {};
+  this.namespace = namespace;
 }
 
 APIFunctions.prototype.register = function(apiName, apiFunction) {
@@ -49,7 +52,17 @@ APIFunctions.prototype.setHook_ =
 
 APIFunctions.prototype.setHandleRequest =
     function(apiName, customizedFunction) {
-  return this.setHook_(apiName, 'handleRequest', customizedFunction);
+  var prefix = this.namespace;
+  return this.setHook_(apiName, 'handleRequest',
+    function() {
+      var ret = customizedFunction.apply(this, arguments);
+      // Logs API calls to the Activity Log if it doesn't go through an
+      // ExtensionFunction.
+      if (!sendRequestHandler.getCalledSendRequest())
+        logActivity(extensionId, prefix + "." + apiName,
+            Array.prototype.slice.call(arguments));
+      return ret;
+    });
 };
 
 APIFunctions.prototype.setUpdateArgumentsPostValidate =
@@ -135,7 +148,7 @@ var platform = getPlatform();
 
 function Binding(schema) {
   this.schema_ = schema;
-  this.apiFunctions_ = new APIFunctions();
+  this.apiFunctions_ = new APIFunctions(schema.namespace);
   this.customEvent_ = null;
   this.customHooks_ = [];
 };
@@ -288,6 +301,8 @@ Binding.prototype = {
           if (this.updateArgumentsPostValidate)
             args = this.updateArgumentsPostValidate.apply(this, args);
 
+          sendRequestHandler.clearCalledSendRequest();
+
           var retval;
           if (this.handleRequest) {
             retval = this.handleRequest.apply(this, args);
@@ -299,6 +314,7 @@ Binding.prototype = {
                                  this.definition.parameters,
                                  optArgs);
           }
+          sendRequestHandler.clearCalledSendRequest();
 
           // Validate return value if defined - only in debug.
           if (chromeHidden.validateCallbacks &&

chrome/renderer/resources/extensions/send_request.js

@@ -13,6 +13,10 @@ var validate = require('schemaUtils').validate;
 // All outstanding requests from sendRequest().
 var requests = {};
 
+// Used to prevent double Activity Logging for API calls that use both custom
+// bindings and ExtensionFunctions (via sendRequest).
+var calledSendRequest = false;
+
 // Callback handling.
 chromeHidden.handleResponse = function(requestId, name,
                                        success, responseList, error) {
@@ -103,6 +107,7 @@ function prepareRequest(args, argSchemas) {
 //   thread.
 // - preserveNullInObjects: true if it is safe for null to be in objects.
 function sendRequest(functionName, args, argSchemas, optArgs) {
+  calledSendRequest = true;
   if (!optArgs)
     optArgs = {};
   var request = prepareRequest(args, argSchemas);
@@ -134,4 +139,14 @@ function sendRequest(functionName, args, argSchemas, optArgs) {
                         optArgs.preserveNullInObjects);
 }
 
+function getCalledSendRequest() {
+  return calledSendRequest;
+}
+
+function clearCalledSendRequest() {
+  calledSendRequest = false;
+}
+
 exports.sendRequest = sendRequest;
+exports.getCalledSendRequest = getCalledSendRequest;
+exports.clearCalledSendRequest = clearCalledSendRequest;

chrome/test/data/extensions/activity_log/manifest.json

@@ -5,5 +5,6 @@
   "permissions": [ "cookies", "tabs", "webRequest", "webRequestBlocking",
                    "http://*/*", "https://*/*" ],
   "options_page": "options.html",
-  "manifest_version": 2
+  "manifest_version": 2,
+  "omnibox": { "keyword" : "hello" }
 }

chrome/test/data/extensions/activity_log/options.html

@@ -5,6 +5,8 @@
   </head>
   <body>
     <button id="api_call">Make a successful API call</button><br />
+    <button id="special_call">Make a special API call</button><br />
+    <button id="double">Check we don't double log API calls</button><br />
     <button id="blocked_call">Make a blocked API call</button><br />
     <button id="inject_cs">Inject a content script</button><br />
     <button id="inject_blob">Inject a blob of code</button><br />

chrome/test/data/extensions/activity_log/options.js

@@ -54,6 +54,20 @@ function makeApiCall() {
   setCompleted('makeApiCall');
 }
 
+// Makes an API call that has a custom binding.
+function makeSpecialApiCalls() {
+  var url = chrome.extension.getURL("image/cat.jpg");
+  var noparam = chrome.extension.getViews();
+  setCompleted('makeSpecialApiCalls');
+}
+
+// Checks that we don't double-log calls that go through setHandleRequest
+// *and* the ExtensionFunction machinery.
+function checkNoDoubleLogging() {
+  chrome.omnibox.setDefaultSuggestion({description: 'hello world'});
+  setCompleted('checkNoDoubleLogging');
+}
+
 // Makes an API call that the extension doesn't have permission for.
 function makeBlockedApiCall() {
   try {
@@ -164,12 +178,14 @@ function doWebRequestModifications() {
 // Attach the tests to buttons.
 function setupEvents() {
   $('api_call').addEventListener('click', makeApiCall);
+  $('special_call').addEventListener('click', makeSpecialApiCalls);
   $('blocked_call').addEventListener('click', makeBlockedApiCall);
   $('inject_cs').addEventListener('click', injectContentScript);
   $('inject_blob').addEventListener('click', injectScriptBlob);
   $('background_xhr').addEventListener('click', doBackgroundXHR);
   $('cs_xhr').addEventListener('click', doContentScriptXHR);
   $('webrequest').addEventListener('click', doWebRequestModifications);
+  $('double').addEventListener('click', checkNoDoubleLogging);
 
   completed = 0;
   total = document.getElementsByTagName('button').length;