drive: Add information about in-flight operations to chrome:drive-internals

drive: Add information about in-flight operations to chrome:drive-internals

Adding a section about in-flight operations in drive-internals.
The section is added just below the "Authentication Status" as the opeations seem to be more dynamic than GCache/File System Contents and might be more interesting to the developers.

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

[satorux1, 2012-08-22 20:20:02]

http://codereview.chromium.org/10828422/diff/7001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/drive_internals.js (right):

http://codereview.chromium.org/10828422/diff/7001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:83: * @param {Object}
operationsSummary Dictionaty describing the status of
seems to me that operationsSummary is a list of dictonaries?

http://codereview.chromium.org/10828422/diff/7001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:103: }
I think we want to be able to monitor the in-flight operations without reloading
the page.

Can you add a timer on JavaScript side, and fetch the latest status from the C++
side, and update the section every second?

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

http://codereview.chromium.org/10828422/diff/7001/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:242:
UpdateInFlightOperations(drive_service);
as mentioned in .js file, I think we should update the section every second. To
do that, we should fetch the latest status from JavaScript side. I guess we
don't have to initiate the update of this section from C++ side.

[Haruki Sato, 2012-08-24 19:06:10]

thank you for the thorough review!

http://codereview.chromium.org/10828422/diff/7001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/drive_internals.js (right):

http://codereview.chromium.org/10828422/diff/7001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:83: * @param {Object}
operationsSummary Dictionaty describing the status of
On 2012/08/22 20:20:02, satorux1 wrote:
> seems to me that operationsSummary is a list of dictonaries?

Yes. Done.
Thanks!

http://codereview.chromium.org/10828422/diff/7001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:103: }
On 2012/08/22 20:20:02, satorux1 wrote:
> I think we want to be able to monitor the in-flight operations without
reloading
> the page.
> 
> Can you add a timer on JavaScript side, and fetch the latest status from the
C++
> side, and update the section every second?

Done.
Added periodic update using setInterval.

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

http://codereview.chromium.org/10828422/diff/7001/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:242:
UpdateInFlightOperations(drive_service);
On 2012/08/22 20:20:02, satorux1 wrote:
> as mentioned in .js file, I think we should update the section every second.
To
> do that, we should fetch the latest status from JavaScript side. I guess we
> don't have to initiate the update of this section from C++ side.

Done.

[satorux1, 2012-08-24 22:46:26]

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

http://codereview.chromium.org/10828422/diff/5004/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:412: for
(std::vector<gdata::OperationRegistry::ProgressStatus>::iterator i
i is not used for iterators (iter is usually used).

For a vector, for (size_t i = 0; i < v.size(); ++i) is just more succinct, and I
don't see a point of using the lengthy iterator notation, though some people
disagree.

[Haruki Sato, 2012-08-28 23:12:56]

Sorry for the delay.

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

http://codereview.chromium.org/10828422/diff/5004/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:412: for
(std::vector<gdata::OperationRegistry::ProgressStatus>::iterator i
On 2012/08/24 22:46:26, satorux1 wrote:
> i is not used for iterators (iter is usually used).
> 
> For a vector, for (size_t i = 0; i < v.size(); ++i) is just more succinct, and
I
> don't see a point of using the lengthy iterator notation, though some people
> disagree. 

Done.
Thanks.

[satorux1, 2012-08-28 23:19:10]

LGTM with a nit. Besides please update the patch description.

The first line looks awkward:


> Implement http://crbug.com/140547: Add information about in-flight operations
to chrome:drive-internals 

Consider:

drive: Add information about in-flight operations to chrome:drive-internals

http://codereview.chromium.org/10828422/diff/16001/chrome/browser/ui/webui/ch...
File chrome/browser/ui/webui/chromeos/drive_internals_ui.cc (right):

http://codereview.chromium.org/10828422/diff/16001/chrome/browser/ui/webui/ch...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:413:
gdata::OperationRegistry::ProgressStatus status = progress_status_list[i];
I think you can make it a const ref

const gdata::OperationRegistry::ProgressStatus* status =
    progress_status_list[i];

[Haruki Sato, 2012-08-29 17:15:45]

Added const and updated the change description.

http://codereview.chromium.org/10828422/diff/16001/chrome/browser/ui/webui/ch...
File chrome/browser/ui/webui/chromeos/drive_internals_ui.cc (right):

http://codereview.chromium.org/10828422/diff/16001/chrome/browser/ui/webui/ch...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:413:
gdata::OperationRegistry::ProgressStatus status = progress_status_list[i];
On 2012/08/28 23:19:10, satorux1 wrote:
> I think you can make it a const ref
> 
> const gdata::OperationRegistry::ProgressStatus* status =
>     progress_status_list[i];

Done.
Thanks.

[commit-bot: I haz the power, 2012-08-29 20:27:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haruki@chromium.org/10828422/22001

[commit-bot: I haz the power, 2012-08-30 03:12:15]

Change committed as 154056