drive: Event logger for chrome:drive-internals.

drive: Event logger for chrome:drive-internals.

BUG=158005
TEST=Open chrome:drive-internals, and verify "Event Log" section is added.
TBR=sky@chromium.org

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

[satorux1, 2012-11-14 05:55:11]

http://codereview.chromium.org/11365249/diff/1009/chrome/browser/chromeos/dri...
File chrome/browser/chromeos/drive/event_logger.h (right):

http://codereview.chromium.org/11365249/diff/1009/chrome/browser/chromeos/dri...
chrome/browser/chromeos/drive/event_logger.h:16: class EventLogger {
Class comment is missing.

http://codereview.chromium.org/11365249/diff/1009/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/drive_internals_ui.cc (right):

http://codereview.chromium.org/11365249/diff/1009/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:553: log[i].what ==
last_sent_event_.what) {
This looks tricky. What about introducing a monotonically increasing event ID,
and do:

for (size_t i = 0; i < log.size(); ++i) {
  // Skip events which were already sent.
  if (log[i].id < last_event_id)
    continue;
  ...
}

[satorux1, 2012-11-14 05:55:51]

On 2012/11/14 05:55:11, satorux1 wrote:
>
http://codereview.chromium.org/11365249/diff/1009/chrome/browser/chromeos/dri...
> File chrome/browser/chromeos/drive/event_logger.h (right):
> 
>
http://codereview.chromium.org/11365249/diff/1009/chrome/browser/chromeos/dri...
> chrome/browser/chromeos/drive/event_logger.h:16: class EventLogger {
> Class comment is missing.
> 
>
http://codereview.chromium.org/11365249/diff/1009/chrome/browser/ui/webui/chr...
> File chrome/browser/ui/webui/chromeos/drive_internals_ui.cc (right):
> 
>
http://codereview.chromium.org/11365249/diff/1009/chrome/browser/ui/webui/chr...
> chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:553: log[i].what ==
> last_sent_event_.what) {
> This looks tricky. What about introducing a monotonically increasing event ID,
> and do:
> 
> for (size_t i = 0; i < log.size(); ++i) {
>   // Skip events which were already sent.
>   if (log[i].id < last_event_id)

<=

>     continue;
>   ...
> }

[kinaba, 2012-11-14 06:19:52]

http://codereview.chromium.org/11365249/diff/1009/chrome/browser/chromeos/dri...
File chrome/browser/chromeos/drive/event_logger.h (right):

http://codereview.chromium.org/11365249/diff/1009/chrome/browser/chromeos/dri...
chrome/browser/chromeos/drive/event_logger.h:16: class EventLogger {
On 2012/11/14 05:55:11, satorux1 wrote:
> Class comment is missing.

Done.

http://codereview.chromium.org/11365249/diff/1009/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/drive_internals_ui.cc (right):

http://codereview.chromium.org/11365249/diff/1009/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:553: log[i].what ==
last_sent_event_.what) {
On 2012/11/14 05:55:11, satorux1 wrote:
> This looks tricky. What about introducing a monotonically increasing event ID,
> and do:
> 
> for (size_t i = 0; i < log.size(); ++i) {
>   // Skip events which were already sent.
>   if (log[i].id < last_event_id)
>     continue;
>   ...
> }

Done.

[satorux1, 2012-11-14 07:59:30]

LGTM with nits

http://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dri...
File chrome/browser/chromeos/drive/event_logger.cc (right):

http://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dri...
chrome/browser/chromeos/drive/event_logger.cc:24:
history_.push_back(Event(next_event_id_++, what));
Let's increment this on the next line. post-increment semantic is a bit tricky. 

history_.push_back(Event(next_event_id_, what));
++next_event_id;

http://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dri...
File chrome/browser/chromeos/drive/event_logger.h (right):

http://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dri...
chrome/browser/chromeos/drive/event_logger.h:23: int id;
Please document that this is a monotonically increasing counter which starts
from 0.

http://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dri...
chrome/browser/chromeos/drive/event_logger.h:24: base::Time when;
;  // When the event occurred.

http://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dri...
chrome/browser/chromeos/drive/event_logger.h:25: std::string what;
;  // What happened.

[kinaba, 2012-11-14 08:13:35]

https://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dr...
File chrome/browser/chromeos/drive/event_logger.cc (right):

https://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/event_logger.cc:24:
history_.push_back(Event(next_event_id_++, what));
On 2012/11/14 07:59:30, satorux1 wrote:
> Let's increment this on the next line. post-increment semantic is a bit
tricky. 
> 
> history_.push_back(Event(next_event_id_, what));
> ++next_event_id;

Done.

https://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dr...
File chrome/browser/chromeos/drive/event_logger.h (right):

https://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/event_logger.h:23: int id;
On 2012/11/14 07:59:30, satorux1 wrote:
> Please document that this is a monotonically increasing counter which starts
> from 0.

Done.

https://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/event_logger.h:24: base::Time when;
On 2012/11/14 07:59:30, satorux1 wrote:
> ;  // When the event occurred. 

Done.

https://codereview.chromium.org/11365249/diff/8001/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/event_logger.h:25: std::string what;
On 2012/11/14 07:59:30, satorux1 wrote:
> ;  // What happened.

Done.

[kinaba, 2012-11-14 09:54:55]

TRBing sky@ for chrome/*.gyp change.

[commit-bot: I haz the power, 2012-11-14 09:55:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinaba@chromium.org/11365249/12003

[commit-bot: I haz the power, 2012-11-14 10:59:35]

Change committed as 167647