Chrome diagnostics: First cut at implementing chrome://diagnostics.

Chrome diagnostics: First cut at implementing chrome://diagnostics.

BUG=139442
TEST=chrome://diagnostics


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

[James Hawkins, 2012-08-01 00:47:42]

http://codereview.chromium.org/10836039/diff/1/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/diagnostics/main.css (right):

http://codereview.chromium.org/10836039/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/diagnostics/main.css:6: /* To be implemented.
crbug.com/139442
Don't add this file until there's content.

http://codereview.chromium.org/10836039/diff/1/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/diagnostics/main.js (right):

http://codereview.chromium.org/10836039/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/diagnostics/main.js:5: // To be implemented.
crbug.com/139442
Don't add this file until there's content.

http://codereview.chromium.org/10836039/diff/1/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc (right):

http://codereview.chromium.org/10836039/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc:33: //
WebUIMessageHandler override.
s/override/implementation/

http://codereview.chromium.org/10836039/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc:51: // TODO:
invoke debugd methods to retrieve diagnostics information, and
s/debugd/debug/

...right?

[hshi1, 2012-08-01 00:57:10]

http://codereview.chromium.org/10836039/diff/1/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/diagnostics/main.css (right):

http://codereview.chromium.org/10836039/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/diagnostics/main.css:6: /* To be implemented.
crbug.com/139442
On 2012/08/01 00:47:42, James Hawkins wrote:
> Don't add this file until there's content.

Done.

http://codereview.chromium.org/10836039/diff/1/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/diagnostics/main.js (right):

http://codereview.chromium.org/10836039/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/diagnostics/main.js:5: // To be implemented.
crbug.com/139442
On 2012/08/01 00:47:42, James Hawkins wrote:
> Don't add this file until there's content.

Done.

http://codereview.chromium.org/10836039/diff/1/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc (right):

http://codereview.chromium.org/10836039/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc:33: //
WebUIMessageHandler override.
On 2012/08/01 00:47:42, James Hawkins wrote:
> s/override/implementation/

Done.

http://codereview.chromium.org/10836039/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc:51: // TODO:
invoke debugd methods to retrieve diagnostics information, and
Actually no the 'debugd' here refers to the debug daemon (often abbreviated as
debugd). What needs to happen here is that upon page loaded, the
DiagnosticsWebUIHandler should kick off a debugd method to get network
interfaces info asynchronously, and upon completion call a javascript function
to display the updated information. The call would look somewhat like this:

chromeos::DBusThreadManager::Get()->GetDebugDaemonClient()->
GetNetworkInterfaces(base::Bind(&DiagnosticsWebUIHandler::OnGetNetworkInterfaces,
weak_ptr_factory_.GetWeakPtr());

For the sake of clarify I plan to get the "first-cut" landed first, then
implement the functionality in a later CL.

On 2012/08/01 00:47:42, James Hawkins wrote:
> s/debugd/debug/
> 
> ...right?

[hshi1, 2012-08-01 18:12:57]

Please take a look at Patch Set 3. Thank you!

[hshi1, 2012-08-01 21:01:39]

Ping. Please let me know if you're too busy now, thanks!

[James Hawkins, 2012-08-01 21:46:05]

I won't be able to look at this today, but I can tomorrow.

[hshi1, 2012-08-01 21:50:50]

Add ellyjones@ as she is more familiar with the debug daemon. (But in any case,
this patch itself only adds a blank chrome://diagnostics page with zero
functionality.)

[Elly Fong-Jones, 2012-08-01 21:52:19]

On 2012/08/01 21:50:50, hshi1 wrote:
> Add ellyjones@ as she is more familiar with the debug daemon. (But in any
case,
> this patch itself only adds a blank chrome://diagnostics page with zero
> functionality.)

I don't have anything to say about this CL, but thanks for the heads-up.

[hshi1, 2012-08-02 17:48:07]

Pinging again; please review this today if you have time, thanks!

In the latest patch I have already addressed the earlier comments by jhawkins@.

[James Hawkins, 2012-08-02 18:30:08]

LGTM with nits.

http://codereview.chromium.org/10836039/diff/6006/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc (right):

http://codereview.chromium.org/10836039/diff/6006/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc:30: virtual
~DiagnosticsWebUIHandler() { }
nit: {}

http://codereview.chromium.org/10836039/diff/6006/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.h (right):

http://codereview.chromium.org/10836039/diff/6006/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.h:22: } // namespace
chromeos
nit: Two spaces before start of comment.

[hshi1, 2012-08-02 18:37:19]

http://codereview.chromium.org/10836039/diff/6006/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc (right):

http://codereview.chromium.org/10836039/diff/6006/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.cc:30: virtual
~DiagnosticsWebUIHandler() { }
On 2012/08/02 18:30:08, James Hawkins wrote:
> nit: {}

Done.

http://codereview.chromium.org/10836039/diff/6006/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.h (right):

http://codereview.chromium.org/10836039/diff/6006/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/diagnostics/diagnostics_ui.h:22: } // namespace
chromeos
On 2012/08/02 18:30:08, James Hawkins wrote:
> nit: Two spaces before start of comment.

Done.

[commit-bot: I haz the power, 2012-08-02 18:38:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hshi@chromium.org/10836039/5005

[commit-bot: I haz the power, 2012-08-02 19:40:15]

Change committed as 149676