Add a console interface for logging to the JS console from a PPAPI plugin....

Add a console interface for logging to the JS console from a PPAPI plugin.

TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=77852

[yzshen1, 2011-03-11 00:40:35]

http://codereview.chromium.org/6667010/diff/6003/ppapi/c/dev/ppb_console_dev.h
File ppapi/c/dev/ppb_console_dev.h (right):

http://codereview.chromium.org/6667010/diff/6003/ppapi/c/dev/ppb_console_dev....
ppapi/c/dev/ppb_console_dev.h:5: #ifndef PPAPI_C_DEV_PPB_CONSOLE_H_
I think PPAPI_C_DEV_PPB_CONSOLE_DEV_H_ would be more consistent with other files
in this folder.

http://codereview.chromium.org/6667010/diff/6003/ppapi/c/dev/ppb_console_dev....
ppapi/c/dev/ppb_console_dev.h:22: * Logs a the given message to the JavaScript
console associated with the
Please remove "a".

http://codereview.chromium.org/6667010/diff/6003/ppapi/c/dev/ppb_console_dev....
ppapi/c/dev/ppb_console_dev.h:24: * issuing the log message will be
automatically pre-pended to the message.
pre-pended -> prepended, please.

http://codereview.chromium.org/6667010/diff/6003/ppapi/proxy/ppb_console_prox...
File ppapi/proxy/ppb_console_proxy.cc (right):

http://codereview.chromium.org/6667010/diff/6003/ppapi/proxy/ppb_console_prox...
ppapi/proxy/ppb_console_proxy.cc:10: #include "ppapi/proxy/serialized_var.h"
This has already been included in the .h file, I think we may not need it.

http://codereview.chromium.org/6667010/diff/6003/ppapi/proxy/ppb_console_prox...
ppapi/proxy/ppb_console_proxy.cc:87: static_cast<PP_LogLevel_Dev>(log_level),
Wrong indent.

http://codereview.chromium.org/6667010/diff/6003/ppapi/proxy/ppb_console_proxy.h
File ppapi/proxy/ppb_console_proxy.h (right):

http://codereview.chromium.org/6667010/diff/6003/ppapi/proxy/ppb_console_prox...
ppapi/proxy/ppb_console_proxy.h:18: class SerializedVarReturnValue;
Why do we need this forward declaration?

http://codereview.chromium.org/6667010/diff/6003/webkit/plugins/ppapi/ppb_con...
File webkit/plugins/ppapi/ppb_console_impl.cc (right):

http://codereview.chromium.org/6667010/diff/6003/webkit/plugins/ppapi/ppb_con...
webkit/plugins/ppapi/ppb_console_impl.cc:66:
ReplaceSubstringsAfterOffset(&message, 0, null, "\\0");
We have duplicate code for escaping NULLs, and we have run it for |value|
already.

Can we consider to use Var::PPVarToLogString() for |source| as well?

http://codereview.chromium.org/6667010/diff/6003/webkit/plugins/ppapi/ppb_con...
File webkit/plugins/ppapi/ppb_console_impl.h (right):

http://codereview.chromium.org/6667010/diff/6003/webkit/plugins/ppapi/ppb_con...
webkit/plugins/ppapi/ppb_console_impl.h:15: // Returns a pointer to the
interface implementing PPB_Console that is
PPB_Console -> PPB_Console_Dev, please.

[brettw, 2011-03-11 19:09:21]

New snap up, thanks for the thorough review.

[yzshen1, 2011-03-11 19:35:25]

LGTM

Thanks!

On 2011/03/11 19:09:21, brettw wrote:
> New snap up, thanks for the thorough review.