Chromium Code Reviews| Index: chrome/browser/extensions/extension_function_dispatcher.cc |
| =================================================================== |
| --- chrome/browser/extensions/extension_function_dispatcher.cc (revision 173817) |
| +++ chrome/browser/extensions/extension_function_dispatcher.cc (working copy) |
| @@ -6,6 +6,7 @@ |
| #include <map> |
| +#include "base/bind.h" |
| #include "base/json/json_string_value_serializer.h" |
| #include "base/lazy_instance.h" |
| #include "base/memory/ref_counted.h" |
| @@ -27,6 +28,7 @@ |
| #include "chrome/common/extensions/extension_messages.h" |
| #include "chrome/common/extensions/extension_set.h" |
| #include "chrome/common/url_constants.h" |
| +#include "content/public/browser/browser_thread.h" |
| #include "content/public/browser/render_process_host.h" |
| #include "content/public/browser/render_view_host.h" |
| #include "ipc/ipc_message.h" |
| @@ -45,41 +47,59 @@ |
| const char kQuotaExceeded[] = "quota exceeded"; |
| void LogSuccess(const Extension* extension, |
| - const ExtensionHostMsg_Request_Params& params) { |
| - extensions::ActivityLog* activity_log = |
| - extensions::ActivityLog::GetInstance(); |
| - if (activity_log->HasObservers(extension)) { |
| - std::string call_signature = params.name + "("; |
| - ListValue::const_iterator it = params.arguments.begin(); |
| - for (; it != params.arguments.end(); ++it) { |
| - std::string arg; |
| - JSONStringValueSerializer serializer(&arg); |
| - if (serializer.SerializeAndOmitBinaryValues(**it)) { |
| - if (it != params.arguments.begin()) |
| - call_signature += ", "; |
| - call_signature += arg; |
| - } |
| - } |
| - call_signature += ")"; |
| - |
| - activity_log->Log(extension, |
| - extensions::ActivityLog::ACTIVITY_EXTENSION_API_CALL, |
| - call_signature); |
| + const std::string& api_name, |
| + const ListValue* args, |
|
Matt Perry
2013/01/03 20:53:35
This leaks |args|. Pass as a scoped_ptr to fix.
felt
2013/01/07 23:44:22
I think I've done this correctly, assuming you don
Matt Perry
2013/01/08 00:02:01
Correct. (Only use scoped_ptr when passing ownersh
|
| + Profile* profile) { |
| + // 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. |
| + BrowserThread::ID thread_id; |
| + bool known = BrowserThread::GetCurrentThreadIdentifier(&thread_id); |
|
Matt Perry
2013/01/03 20:53:35
Use BrowserThread::CurrentlyOn
felt
2013/01/07 23:44:22
Done.
|
| + if (!known || thread_id != BrowserThread::UI) { |
| + BrowserThread::PostTask(BrowserThread::UI, |
| + FROM_HERE, |
| + base::Bind(&LogSuccess, |
| + extension, |
| + api_name, |
| + args, |
| + profile)); |
| + } else { |
| + extensions::ActivityLog* activity_log = |
| + extensions::ActivityLog::GetInstance(profile); |
| + if (activity_log->HasObservers(extension)) |
| + activity_log->LogAPIAction(extension, api_name, args); |
| } |
| } |
| void LogFailure(const Extension* extension, |
| - const std::string& func_name, |
| - const char* reason) { |
| - extensions::ActivityLog* activity_log = |
| - extensions::ActivityLog::GetInstance(); |
| - if (activity_log->HasObservers(extension)) { |
| - activity_log->Log(extension, |
| - extensions::ActivityLog::ACTIVITY_EXTENSION_API_BLOCK, |
| - func_name + ": " + reason); |
| + const std::string& api_name, |
| + const ListValue* args, |
| + const char* reason, |
| + Profile* profile) { |
| + // 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. |
| + BrowserThread::ID thread_id; |
| + bool known = BrowserThread::GetCurrentThreadIdentifier(&thread_id); |
| + if (!known || thread_id != BrowserThread::UI) { |
| + BrowserThread::PostTask(BrowserThread::UI, |
| + FROM_HERE, |
| + base::Bind(&LogFailure, |
| + extension, |
| + api_name, |
| + args, |
| + reason, |
| + profile)); |
| + } else { |
| + extensions::ActivityLog* activity_log = |
| + extensions::ActivityLog::GetInstance(profile); |
| + if (activity_log->HasObservers(extension)) |
| + activity_log->LogBlockedAction(extension, |
| + api_name, |
| + args, |
| + reason); |
| } |
| } |
| + |
| // Separate copy of ExtensionAPI used for IO thread extension functions. We need |
| // this because ExtensionAPI has mutable data. It should be possible to remove |
| // this once all the extension APIs are updated to the feature system. |
| @@ -129,7 +149,7 @@ |
| const ExtensionHostMsg_Request_Params& params) { |
| const Extension* extension = |
| extension_info_map->extensions().GetByID(params.extension_id); |
| - |
| + Profile* profile_cast = static_cast<Profile*>(profile); |
| scoped_refptr<ExtensionFunction> function( |
| CreateExtensionFunction(params, extension, render_process_id, |
| extension_info_map->process_map(), |
| @@ -137,7 +157,11 @@ |
| profile, |
| ipc_sender, NULL, routing_id)); |
| if (!function) { |
| - LogFailure(extension, params.name, kAccessDenied); |
| + LogFailure(extension, |
| + params.name, |
| + params.arguments.DeepCopy(), |
| + kAccessDenied, |
| + profile_cast); |
| return; |
| } |
| @@ -153,7 +177,11 @@ |
| extension_info_map->IsIncognitoEnabled(extension->id())); |
| if (!CheckPermissions(function, extension, params, ipc_sender, routing_id)) { |
| - LogFailure(extension, params.name, kAccessDenied); |
| + LogFailure(extension, |
| + params.name, |
| + params.arguments.DeepCopy(), |
| + kAccessDenied, |
| + profile_cast); |
| return; |
| } |
| @@ -163,11 +191,18 @@ |
| ¶ms.arguments, |
| base::TimeTicks::Now()); |
| if (violation_error.empty()) { |
| + LogSuccess(extension, |
| + params.name, |
| + params.arguments.DeepCopy(), |
| + profile_cast); |
| function->Run(); |
| - LogSuccess(extension, params); |
| } else { |
| + LogFailure(extension, |
| + params.name, |
| + params.arguments.DeepCopy(), |
| + kQuotaExceeded, |
| + profile_cast); |
| function->OnQuotaExceeded(violation_error); |
| - LogFailure(extension, params.name, kQuotaExceeded); |
| } |
| } |
| @@ -205,7 +240,11 @@ |
| profile(), render_view_host, render_view_host, |
| render_view_host->GetRoutingID())); |
| if (!function) { |
| - LogFailure(extension, params.name, kAccessDenied); |
| + LogFailure(extension, |
| + params.name, |
| + params.arguments.DeepCopy(), |
| + kAccessDenied, |
| + profile()); |
| return; |
| } |
| @@ -221,7 +260,11 @@ |
| if (!CheckPermissions(function, extension, params, render_view_host, |
| render_view_host->GetRoutingID())) { |
| - LogFailure(extension, params.name, kAccessDenied); |
| + LogFailure(extension, |
| + params.name, |
| + params.arguments.DeepCopy(), |
| + kAccessDenied, |
| + profile()); |
| return; |
| } |
| @@ -233,12 +276,15 @@ |
| if (violation_error.empty()) { |
| // See crbug.com/39178. |
| ExternalProtocolHandler::PermitLaunchUrl(); |
| - |
| + LogSuccess(extension, params.name, params.arguments.DeepCopy(), profile()); |
| function->Run(); |
| - LogSuccess(extension, params); |
| } else { |
| + LogFailure(extension, |
| + params.name, |
| + params.arguments.DeepCopy(), |
| + kQuotaExceeded, |
| + profile()); |
| function->OnQuotaExceeded(violation_error); |
| - LogFailure(extension, params.name, kQuotaExceeded); |
| } |
| // Note: do not access |this| after this point. We may have been deleted |