gdata: Add GCache Contents section to chrome:drive-internals

gdata: Add GCache Contents section to chrome:drive-internals

This section is used to show contents in the GCache directory.

BUG=135328
TEST=the new section is shown in chrome:drive-internals properly

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=148752

[satorux1, 2012-07-26 14:05:04]

achuith: changes in .cc/html/js/css files
jhawkins: chrome/browser/browser_resources.grd but please feel free to look at
other changes too.

[James Hawkins, 2012-07-26 17:50:02]

LGTM with nits.

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/drive_internals.html (right):

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:13: <table>
I'm not going to ask you to do this in this CL, this should not be using table
(it's not tabular data).  Please add a TODO to remove the use of table.

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:15: <tr><td>Has refresh
token</td><td id='has-refresh-token'></td></tr>
nit: Drop <td> and co to next line, indented.

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:21: <tbody
id='gcache-contents'>
nit: Move the </tbody> to this line since it's fits w/in 80 cols.

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

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:16: * @param {Array}
gcache_contents List of dictionaries describing metadata
nit: gcacheContents

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:28: for (var i = 0; i <
gcache_contents.length; ++i) {
nit: i++

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:47: * Creates an element of
|elementName| from |text|.
nit: s/of/named/

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:47: * Creates an element of
|elementName| from |text|.
nit: s/from/containing the content/

[satorux1, 2012-07-26 19:18:14]

Thank you for taking a look at the webui stuff.

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/drive_internals.html (right):

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:13: <table>
On 2012/07/26 17:50:02, James Hawkins wrote:
> I'm not going to ask you to do this in this CL, this should not be using table
> (it's not tabular data).  Please add a TODO to remove the use of table.

Sounds reasonable. I'd rather fix now. :)

For now, I changed it to simple <ul>+<li>. Originally, I chose to use table to
make the key value pairs vertically aligned. What would be a recommended way to
do it without a table?

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:15: <tr><td>Has refresh
token</td><td id='has-refresh-token'></td></tr>
On 2012/07/26 17:50:02, James Hawkins wrote:
> nit: Drop <td> and co to next line, indented.

the table is gone now.

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:21: <tbody
id='gcache-contents'>
On 2012/07/26 17:50:02, James Hawkins wrote:
> nit: Move the </tbody> to this line since it's fits w/in 80 cols.

Done.

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

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:28: for (var i = 0; i <
gcache_contents.length; ++i) {
On 2012/07/26 17:50:02, James Hawkins wrote:
> nit: i++

Done.

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:47: * Creates an element of
|elementName| from |text|.
On 2012/07/26 17:50:02, James Hawkins wrote:
> nit: s/of/named/

Done.

[James Hawkins, 2012-07-26 19:30:41]

SLGTM

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/drive_internals.html (right):

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:13: <table>
On 2012/07/26 19:18:14, satorux1 wrote:
> On 2012/07/26 17:50:02, James Hawkins wrote:
> > I'm not going to ask you to do this in this CL, this should not be using
table
> > (it's not tabular data).  Please add a TODO to remove the use of table.
> 
> Sounds reasonable. I'd rather fix now. :)
> 
> For now, I changed it to simple <ul>+<li>. Originally, I chose to use table to
> make the key value pairs vertically aligned. What would be a recommended way
to
> do it without a table?

Check out table.css in shared/.

http://codereview.chromium.org/10823039/diff/8001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/drive_internals.html (right):

http://codereview.chromium.org/10823039/diff/8001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:18: <table>
You want to remove this table while you're at it?

[satorux1, 2012-07-26 20:29:23]

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/drive_internals.html (right):

http://codereview.chromium.org/10823039/diff/2001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:13: <table>
On 2012/07/26 19:30:41, James Hawkins wrote:
> On 2012/07/26 19:18:14, satorux1 wrote:
> > On 2012/07/26 17:50:02, James Hawkins wrote:
> > > I'm not going to ask you to do this in this CL, this should not be using
> table
> > > (it's not tabular data).  Please add a TODO to remove the use of table.
> > 
> > Sounds reasonable. I'd rather fix now. :)
> > 
> > For now, I changed it to simple <ul>+<li>. Originally, I chose to use table
to
> > make the key value pairs vertically aligned. What would be a recommended way
> to
> > do it without a table?
> 
> Check out table.css in shared/.

Thanks. I'll take a look.

http://codereview.chromium.org/10823039/diff/8001/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/drive_internals.html (right):

http://codereview.chromium.org/10823039/diff/8001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.html:18: <table>
On 2012/07/26 19:30:41, James Hawkins wrote:
> You want to remove this table while you're at it?

I'd keep this for now.

[satorux1, 2012-07-26 21:17:18]

achuith@, could you take a look at the c++ part?

yoshiki@, this change would be useful for testing your "nuke everything"
feature.

[achuithb, 2012-07-26 23:16:29]

lgtm

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

http://codereview.chromium.org/10823039/diff/8001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:9: function
UpdateAuthStatus(authStatus) {
same

http://codereview.chromium.org/10823039/diff/8001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:19: function
UpdateGCacheContents(gcacheContents) {
I think the first letter needs not be capitalized for function names in
javascript?

[satorux1, 2012-07-27 15:23:23]

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

http://codereview.chromium.org/10823039/diff/8001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:9: function
UpdateAuthStatus(authStatus) {
On 2012/07/26 23:16:29, achuith.bhandarkar wrote:
> same

Done.

http://codereview.chromium.org/10823039/diff/8001/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/drive_internals.js:19: function
UpdateGCacheContents(gcacheContents) {
On 2012/07/26 23:16:29, achuith.bhandarkar wrote:
> I think the first letter needs not be capitalized for function names in
> javascript?

Good catch! Done.