Hook up PA_LOG() logs to chrome://proximity-auth WebUI.

Hook up PA_LOG() logs to chrome://proximity-auth WebUI.

BUG=409158
TEST=manual

Committed: https://crrev.com/dc5cefc1aa1f1adeab07303206a538d9a2441984
Cr-Commit-Position: refs/heads/master@{#329877}

[Tim Song, 2015-05-12 20:51:31]

Patchset #1 (id:1) has been deleted

[Tim Song, 2015-05-12 20:51:38]

Patchset #1 (id:20001) has been deleted

[Tim Song, 2015-05-12 20:51:45]

Patchset #1 (id:40001) has been deleted

[Tim Song, 2015-05-12 20:51:55]

tengs@chromium.org changed reviewers:
+ isherman@chromium.org

[Ilya Sherman, 2015-05-13 00:49:58]

Thanks, Tim.

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/logging/log_buffer.h (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/logging/log_buffer.h:40: virtual void
OnLogBufferCleared() = 0;
Please document these methods.

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.cc (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:24: //
JavaScript functions. The caller owns the returned value.
Please return a scoped_ptr if passing ownership.

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.h (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.h:30: // LogBuffer:
nit: LogBuffer::Observer:

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/webui/resources/log-buffer.js (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/webui/resources/log-buffer.js:54: // for each
operation performed on the native LogBuffer.
nit: "when for each" -> "for each"?

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/webui/resources/log-panel.js (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/webui/resources/log-panel.js:45:
//this.async(this.scrollToBottom_);
nit: Remove this line?

[Tim Song, 2015-05-13 02:24:37]

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/logging/log_buffer.h (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/logging/log_buffer.h:40: virtual void
OnLogBufferCleared() = 0;
On 2015/05/13 00:49:57, Ilya Sherman wrote:
> Please document these methods.

Done.

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.cc (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:24: //
JavaScript functions. The caller owns the returned value.
On 2015/05/13 00:49:57, Ilya Sherman wrote:
> Please return a scoped_ptr if passing ownership.

Done.

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.h (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.h:30: // LogBuffer:
On 2015/05/13 00:49:57, Ilya Sherman wrote:
> nit: LogBuffer::Observer:

Done.

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/webui/resources/log-buffer.js (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/webui/resources/log-buffer.js:54: // for each
operation performed on the native LogBuffer.
On 2015/05/13 00:49:57, Ilya Sherman wrote:
> nit: "when for each" -> "for each"?

Done.

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
File components/proximity_auth/webui/resources/log-panel.js (right):

https://codereview.chromium.org/1135293002/diff/60001/components/proximity_au...
components/proximity_auth/webui/resources/log-panel.js:45:
//this.async(this.scrollToBottom_);
On 2015/05/13 00:49:57, Ilya Sherman wrote:
> nit: Remove this line?

Done.

[Ilya Sherman, 2015-05-13 21:57:10]

LGTM % the remaining nits.  Thanks, Tim.

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.cc (right):

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:27:
base::DictionaryValue* dictionary = new base::DictionaryValue();
nit: Please use a scoped_ptr here, and .Pass() it below.

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:61:
LogMessageToDictionary(log_message).release());
nit: You should be able to just assign the value, or maybe call .Pass() if that
doesn't work.  .release() is dispreferred where more clearly safe options work.

[Tim Song, 2015-05-13 23:26:55]

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.cc (right):

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:27:
base::DictionaryValue* dictionary = new base::DictionaryValue();
On 2015/05/13 21:57:10, Ilya Sherman wrote:
> nit: Please use a scoped_ptr here, and .Pass() it below.

Done.

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:61:
LogMessageToDictionary(log_message).release());
On 2015/05/13 21:57:10, Ilya Sherman wrote:
> nit: You should be able to just assign the value, or maybe call .Pass() if
that
> doesn't work.  .release() is dispreferred where more clearly safe options
work.

I tried both and got compile errors. It seems scoped_ptr instances don't cast to
their pointer values in this case.

[Ilya Sherman, 2015-05-13 23:42:34]

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.cc (right):

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:61:
LogMessageToDictionary(log_message).release());
On 2015/05/13 23:26:55, Tim Song wrote:
> On 2015/05/13 21:57:10, Ilya Sherman wrote:
> > nit: You should be able to just assign the value, or maybe call .Pass() if
> that
> > doesn't work.  .release() is dispreferred where more clearly safe options
> work.
> 
> I tried both and got compile errors. It seems scoped_ptr instances don't cast
to
> their pointer values in this case.

Hmm, according to [
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped...
], either of the following should work:

scoped_ptr<base::DictionaryValue> dictionary =
LogMessageToDictionary(log_message);
scoped_ptr<base::DictionaryValue>
dictionary(LogMessageToDictionary(log_message).Pass());

You tried both and got compile errors?  I'm pretty surprised by that, especially
since I'm pretty sure I've seen this pattern used elsewhere in the code.

[Tim Song, 2015-05-14 03:12:03]

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.cc (right):

https://codereview.chromium.org/1135293002/diff/80001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:61:
LogMessageToDictionary(log_message).release());
On 2015/05/13 23:42:34, Ilya Sherman wrote:
> On 2015/05/13 23:26:55, Tim Song wrote:
> > On 2015/05/13 21:57:10, Ilya Sherman wrote:
> > > nit: You should be able to just assign the value, or maybe call .Pass() if
> > that
> > > doesn't work.  .release() is dispreferred where more clearly safe options
> > work.
> > 
> > I tried both and got compile errors. It seems scoped_ptr instances don't
cast
> to
> > their pointer values in this case.
> 
> Hmm, according to [
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/scoped...
> ], either of the following should work:
> 
> scoped_ptr<base::DictionaryValue> dictionary =
> LogMessageToDictionary(log_message);
> scoped_ptr<base::DictionaryValue>
> dictionary(LogMessageToDictionary(log_message).Pass());
> 
> You tried both and got compile errors?  I'm pretty surprised by that,
especially
> since I'm pretty sure I've seen this pattern used elsewhere in the code.

Sorry, my bad. I thought this comment was referring to the other release() call
below.

[Tim Song, 2015-05-14 17:26:37]

The CQ bit was checked by tengs@chromium.org

[Tim Song, 2015-05-14 17:26:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1135293002/#ps120001
(title: "stop using release()")

[commit-bot: I haz the power, 2015-05-14 17:28:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1135293002/120001

[commit-bot: I haz the power, 2015-05-14 18:06:53]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2015-05-14 18:07:42]

Patchset 4 (id:??) landed as
https://crrev.com/dc5cefc1aa1f1adeab07303206a538d9a2441984
Cr-Commit-Position: refs/heads/master@{#329877}