drive: Add the local largest changestamp to chrome:drive-internals

drive: Add the local largest changestamp to chrome:drive-internals

This information would be useful to invetigate issues where the local
file system metadata is out of sync.

Along the way, replace SetString() with SetDouble() for the largest
changestamp values. The largest change stamp should be small enough to
fit in double without losing precision.

BUG=155473
TEST="Largetst Changestamp (local)" is added in chrome:drive-internals


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

[Haruki Sato, 2012-10-15 07:10:29]

https://codereview.chromium.org/11106009/diff/1/chrome/browser/chromeos/drive...
File chrome/browser/chromeos/drive/drive_file_system.h (right):

https://codereview.chromium.org/11106009/diff/1/chrome/browser/chromeos/drive...
chrome/browser/chromeos/drive/drive_file_system.h:138: virtual
DriveResourceMetadata* GetResourceMetadata() OVERRIDE {
Is this overriding something?

https://codereview.chromium.org/11106009/diff/1/chrome/browser/resources/chro...
File chrome/browser/resources/chromeos/drive_internals.html (right):

https://codereview.chromium.org/11106009/diff/1/chrome/browser/resources/chro...
chrome/browser/resources/chromeos/drive_internals.html:28: <li>Largetst
Changestamp (remote):
s/Largetst/Largest/

Seems like I had a typo here.

https://codereview.chromium.org/11106009/diff/1/chrome/browser/resources/chro...
chrome/browser/resources/chromeos/drive_internals.html:31: <li>Largetst
Changestamp (local):
ditto.

https://codereview.chromium.org/11106009/diff/1/chrome/browser/ui/webui/chrom...
File chrome/browser/ui/webui/chromeos/drive_internals_ui.cc (right):

https://codereview.chromium.org/11106009/diff/1/chrome/browser/ui/webui/chrom...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:13: #include
"base/string_number_conversions.h"
If we don't use Int64ToString in this file,
can we remove this line as well?

https://codereview.chromium.org/11106009/diff/1/chrome/browser/ui/webui/chrom...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:241:
account_metadata.SetDouble("account-largest-changestamp-remote",
I personally like string here as they are considered IDs.
Are we comparing those changestamps to show any message in drive-internals?
e.g.
if (local < remote)
  show("stale. need to sync.")
else
  show("latest. things in sync.")
maybe in JS?

[Zachary Kuznia, 2012-10-15 09:36:55]

The changes to move and remove operation shouldn't be needed once you rebase,
since they're given a pointer to the metadata directly.

[satorux1, 2012-10-16 05:02:56]

http://codereview.chromium.org/11106009/diff/1/chrome/browser/chromeos/drive/...
File chrome/browser/chromeos/drive/drive_file_system.h (right):

http://codereview.chromium.org/11106009/diff/1/chrome/browser/chromeos/drive/...
chrome/browser/chromeos/drive/drive_file_system.h:138: virtual
DriveResourceMetadata* GetResourceMetadata() OVERRIDE {
On 2012/10/15 07:10:29, Haruki Sato wrote:
> Is this overriding something?

yes. one in DriveFileSystemInterface

http://codereview.chromium.org/11106009/diff/1/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/drive_internals.html (right):

http://codereview.chromium.org/11106009/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/drive_internals.html:28: <li>Largetst
Changestamp (remote):
On 2012/10/15 07:10:29, Haruki Sato wrote:
> s/Largetst/Largest/
> 
> Seems like I had a typo here.

Done.

http://codereview.chromium.org/11106009/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/drive_internals.html:31: <li>Largetst
Changestamp (local):
On 2012/10/15 07:10:29, Haruki Sato wrote:
> ditto.

Done.

http://codereview.chromium.org/11106009/diff/1/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chromeos/drive_internals_ui.cc (right):

http://codereview.chromium.org/11106009/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:13: #include
"base/string_number_conversions.h"
On 2012/10/15 07:10:29, Haruki Sato wrote:
> If we don't use Int64ToString in this file,
> can we remove this line as well?

Done. good catch!

http://codereview.chromium.org/11106009/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/drive_internals_ui.cc:241:
account_metadata.SetDouble("account-largest-changestamp-remote",
On 2012/10/15 07:10:29, Haruki Sato wrote:
> I personally like string here as they are considered IDs.

These are not IDs. These are monotonically increasing numbers which can be
compared, hence using double should be fine.


> Are we comparing those changestamps to show any message in drive-internals?
> e.g.
> if (local < remote)
>   show("stale. need to sync.")
> else
>   show("latest. things in sync.")
> maybe in JS?

http://codereview.chromium.org/11106009/diff/6002/chrome/browser/chromeos/dri...
File chrome/browser/chromeos/drive/drive_file_system_interface.h (right):

http://codereview.chromium.org/11106009/diff/6002/chrome/browser/chromeos/dri...
chrome/browser/chromeos/drive/drive_file_system_interface.h:352: virtual
DriveResourceMetadata* GetResourceMetadata() = 0;
Changed my mind. Exposing DriveResourceMetadata is not a good idea. I changed
this to GetMetadata() which returns a struct of the file system metadata.

[Zachary Kuznia, 2012-10-16 07:19:11]

https://codereview.chromium.org/11106009/diff/5008/chrome/browser/chromeos/dr...
File chrome/browser/chromeos/drive/drive_file_system.cc (right):

https://codereview.chromium.org/11106009/diff/5008/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/drive_file_system.cc:2654: DriveFileSystemMetadata
DriveFileSystem::GetMetadata() const {
Since you're only returning the largest changestamp, why not just add an
accessor for that field instead of the entire structure?

[satorux1, 2012-10-16 07:37:16]

https://codereview.chromium.org/11106009/diff/5008/chrome/browser/chromeos/dr...
File chrome/browser/chromeos/drive/drive_file_system.cc (right):

https://codereview.chromium.org/11106009/diff/5008/chrome/browser/chromeos/dr...
chrome/browser/chromeos/drive/drive_file_system.cc:2654: DriveFileSystemMetadata
DriveFileSystem::GetMetadata() const {
On 2012/10/16 07:19:12, Zachary Kuznia wrote:
> Since you're only returning the largest changestamp, why not just add an
> accessor for that field instead of the entire structure?

I'm going to expose some more metadata shortly to chrome:drive-internals. :)

[Zachary Kuznia, 2012-10-16 08:00:13]

lgtm

[Haruki Sato, 2012-10-16 08:24:35]

lgtm

[commit-bot: I haz the power, 2012-10-16 08:44:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/satorux@chromium.org/11106009/5008

[commit-bot: I haz the power, 2012-10-16 10:35:48]

Change committed as 162118