Gets the initial sync service status. In the next patch I'll make the syncfs-internals dashboard a …

Gets the initial sync service status. In the next patch I'll make the syncfs-internals dashboard a sync_event_observer so it can also report changes in real time (as they come by notification).

BUG=226353
TEST=open chrome://syncfs-internals. Service status should be running if signed in. Should be authentication_required if not signed in and any SyncFS app is run (i.e. calls requestFileSystem at least once).

Please not that the page has to be refreshed manually to get updated values for now.

R=arv@chromium.org, kinuko@chromium.org, tzik@chromium.org

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

[calvinlo, 2013-05-02 11:22:13]

Kinuko san, can you please take a quick pass before I send for OWNERS review

[tzik, 2013-05-02 11:47:56]

lgtm

https://codereview.chromium.org/14765003/diff/5001/chrome/browser/resources/s...
File chrome/browser/resources/sync_file_system_internals/sync_service.js
(right):

https://codereview.chromium.org/14765003/diff/5001/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/sync_service.js:25:
$('service-status').innerHTML = status_string;
Can be innerText instead?

https://codereview.chromium.org/14765003/diff/5001/chrome/chrome_browser_ui.gypi
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/14765003/diff/5001/chrome/chrome_browser_ui.g...
chrome/chrome_browser_ui.gypi:2236:
'browser/ui/webui/sync_file_system_internals/sync_file_system_internals_handler.cc',
looks so long...
it's up to the OWNER, but
can we use shorter name like
browser/ui/webui/syncfs/handler.cc? or 
browser/ui/webui/sync_file_system/internals_handler.cc?

[calvinlo, 2013-05-02 11:51:54]

https://codereview.chromium.org/14765003/diff/5001/chrome/browser/resources/s...
File chrome/browser/resources/sync_file_system_internals/sync_service.js
(right):

https://codereview.chromium.org/14765003/diff/5001/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/sync_service.js:25:
$('service-status').innerHTML = status_string;
On 2013/05/02 11:47:56, tzik wrote:
> Can be innerText instead?

Done.

https://codereview.chromium.org/14765003/diff/5001/chrome/chrome_browser_ui.gypi
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/14765003/diff/5001/chrome/chrome_browser_ui.g...
chrome/chrome_browser_ui.gypi:2236:
'browser/ui/webui/sync_file_system_internals/sync_file_system_internals_handler.cc',
I'm not sure on the naming convention here I just followed what
suggestions_internals did. I would like to wait for OWNERs comment first.

[kinuko, 2013-05-02 11:58:41]

I'm not very familiar with webui code, but looks sane to me. The dependency to
extension-generated code worries me a bit, but if you're going to make them more
closely integrated it's probably ok (or I'd defer the decision to relevant
OWNERS) lgtm

[calvinlo, 2013-05-02 12:00:25]

+thestig for chrome_browser_ui.gypi OWNERS review
+arv for browser/resources/*, browser/ui/webui/* OWNERS review

[arv (Not doing code reviews), 2013-05-02 19:45:06]

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
File chrome/browser/resources/sync_file_system_internals/main.html (right):

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/main.html:22: 
Does this really need an id? Can you just use "tabbox" as the selector in your
css?

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
File chrome/browser/resources/sync_file_system_internals/sync_service.js
(right):

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/sync_service.js:23: function
getServiceStatusResult(status_string) {
no underscores in js

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/sync_service.js:25:
$('service-status').innerText = status_string;
textContent

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/sync_service.js:43:
document.addEventListener('DOMContentLoaded', syncService.main);
move into IIFE and skip syncService

document.addEventListener('DOMContentLoaded', main);

Then you probably do not need to export main

[calvinlo, 2013-05-07 06:14:09]

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
File chrome/browser/resources/sync_file_system_internals/main.html (right):

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/main.html:22: 
On 2013/05/02 19:45:06, arv wrote:
> Does this really need an id? Can you just use "tabbox" as the selector in your
> css?

Done.

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
File chrome/browser/resources/sync_file_system_internals/sync_service.js
(right):

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/sync_service.js:23: function
getServiceStatusResult(status_string) {
On 2013/05/02 19:45:06, arv wrote:
> no underscores in js

Done. Changed to camel case.

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/sync_service.js:25:
$('service-status').innerText = status_string;
On 2013/05/02 19:45:06, arv wrote:
> textContent

Done.

https://codereview.chromium.org/14765003/diff/8002/chrome/browser/resources/s...
chrome/browser/resources/sync_file_system_internals/sync_service.js:43:
document.addEventListener('DOMContentLoaded', syncService.main);
On 2013/05/02 19:45:06, arv wrote:
> move into IIFE and skip syncService
> 
> document.addEventListener('DOMContentLoaded', main);
> 
> Then you probably do not need to export main

Done.

[arv (Not doing code reviews), 2013-05-07 21:42:33]

LGTM

[calvinlo, 2013-05-08 09:36:54]

Committed patchset #3 manually as r198867 (presubmit successful).