Chrome Windows Debugger Extension Skeleton

Chrome Windows Debugger Extension Skeleton

This is a skeleton of a Chrome Windows Debugger extension that will
provide convenience commands for developers using ntsd, cdb, or windbg.

BUG=

Review-Url: https://codereview.chromium.org/2612193002
Cr-Commit-Position: refs/heads/master@{#443414}
Committed: https://chromium.googlesource.com/chromium/src/+/fd54b5ccf9c4e391de0bcfe8e852f6784e1a9268

[robliao, 2017-01-05 07:15:22]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 07:15:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[robliao, 2017-01-05 07:58:37]

Patchset #1 (id:1) has been deleted

[robliao, 2017-01-05 08:03:20]

robliao@chromium.org changed reviewers:
+ scottmg@chromium.org

[robliao, 2017-01-05 08:03:21]

scottmg: Please review this CL. Thanks!

[robliao, 2017-01-05 10:15:43]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 10:15:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 11:28:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-05 11:28:29]

Dry run: This issue passed the CQ dry run.

[scottmg, 2017-01-05 15:53:46]

Doesn't seem too Chrome-specific yet, but I assume you have plans to add Aura
details to the HWND, or add more Chrome-y things in the future?

lgtm

https://codereview.chromium.org/2612193002/diff/20001/WATCHLISTS
File WATCHLISTS (right):

https://codereview.chromium.org/2612193002/diff/20001/WATCHLISTS#newcode2057
WATCHLISTS:2057: 'tools_win_chromeexts': ['robliao+watch@chromium.org'],
(Probably unnecessary if you're OWNERS, but up to you.)

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
File tools/win/chromeexts/chromeexts.cc (right):

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
tools/win/chromeexts/chromeexts.cc:32: DEBUG_OUTPUT_NORMAL, "hwnd       -
Displays basic hwnd info.\n");
nit; The number of spaces between hwnd and - seems strange here. Is this to
match alignment from some other output?

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
tools/win/chromeexts/chromeexts.cc:44: hr = debug_control->Evaluate(args,
DEBUG_VALUE_INT64, &value, nullptr);
I assume this will work on x86?

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
tools/win/chromeexts/chromeexts.cc:44: hr = debug_control->Evaluate(args,
DEBUG_VALUE_INT64, &value, nullptr);
Is it necessary to call SetExpressionSyntax(DEBUG_EXPR_CPLUSPLUS)? Or better to
leave as default?

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
tools/win/chromeexts/chromeexts.cc:47: DEBUG_OUTPUT_ERROR,"Unavailable to
evaluate %s\n", args);
Missing space before opening ".

(Run the whole thing though clang-format too).

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
tools/win/chromeexts/chromeexts.cc:47: DEBUG_OUTPUT_ERROR,"Unavailable to
evaluate %s\n", args);
nit; "Unavailable" sounds weird, how about "Unable".

[robliao, 2017-01-06 02:31:06]

Thanks for the review!

> Doesn't seem too Chrome-specific yet, but I assume you have plans to add Aura
>details to the HWND, or add more Chrome-y things in the future?

Yep! That's the plan. I'm going to be adding some of the stuff I wish I had
during debugging as well (like a window message to string converter).

https://codereview.chromium.org/2612193002/diff/20001/WATCHLISTS
File WATCHLISTS (right):

https://codereview.chromium.org/2612193002/diff/20001/WATCHLISTS#newcode2057
WATCHLISTS:2057: 'tools_win_chromeexts': ['robliao+watch@chromium.org'],
On 2017/01/05 15:53:46, scottmg wrote:
> (Probably unnecessary if you're OWNERS, but up to you.)

I've found this useful for ui/display/win when changes get reviewed+approved by
ui/display for changes in ui/display/win.

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
File tools/win/chromeexts/chromeexts.cc (right):

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
tools/win/chromeexts/chromeexts.cc:32: DEBUG_OUTPUT_NORMAL, "hwnd       -
Displays basic hwnd info.\n");
On 2017/01/05 15:53:46, scottmg wrote:
> nit; The number of spaces between hwnd and - seems strange here. Is this to
> match alignment from some other output?

This was a loose convention in !help for other extensions, but I think I'm going
to favor some sort of unified way of doing this. I'll drop this down to one
space in the meantime.

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
tools/win/chromeexts/chromeexts.cc:44: hr = debug_control->Evaluate(args,
DEBUG_VALUE_INT64, &value, nullptr);
On 2017/01/05 15:53:46, scottmg wrote:
> Is it necessary to call SetExpressionSyntax(DEBUG_EXPR_CPLUSPLUS)? Or better
to
> leave as default?

Yep, this works on x86 and x64. Since the upper-bits for an HWND are ignored,
the number of bits for us are less important here. I'll move the comment below
up here.

As for the expression syntax, I think I'd rather leave it default and let
callers change modes manually as necessary. At least I think that's how it
works, the docs leave much to be desired.

https://codereview.chromium.org/2612193002/diff/20001/tools/win/chromeexts/ch...
tools/win/chromeexts/chromeexts.cc:47: DEBUG_OUTPUT_ERROR,"Unavailable to
evaluate %s\n", args);
On 2017/01/05 15:53:46, scottmg wrote:
> Missing space before opening ".
> 
> (Run the whole thing though clang-format too).

Done and done.

[robliao, 2017-01-06 02:31:33]

robliao@chromium.org changed reviewers:
+ brettw@chromium.org

[robliao, 2017-01-06 02:31:33]

brettw@chromium.org: Please review changes in /BUILD.gn. Thanks!

[robliao, 2017-01-06 02:31:48]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 02:33:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 03:32:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 03:32:55]

Dry run: This issue passed the CQ dry run.

[robliao, 2017-01-12 18:47:33]

The CQ bit was checked by robliao@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 18:48:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 20:15:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 20:15:09]

Dry run: This issue passed the CQ dry run.

[brettw, 2017-01-12 22:52:42]

lgtm

[robliao, 2017-01-12 23:02:18]

The CQ bit was checked by robliao@chromium.org

[robliao, 2017-01-12 23:02:18]

The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/2612193002/#ps40001
(title: "CR Feedback")

[commit-bot: I haz the power, 2017-01-12 23:04:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-13 00:41:34]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484262138234240,
"parent_rev": "955b101f8b2f8f1e1db8d80cbac4c3519504ec39", "commit_rev":
"fd54b5ccf9c4e391de0bcfe8e852f6784e1a9268"}

[commit-bot: I haz the power, 2017-01-13 00:42:18]

Description was changed from

==========
Chrome Windows Debugger Extension Skeleton

This is a skeleton of a Chrome Windows Debugger extension that will
provide convenience commands for developers using ntsd, cdb, or windbg.

BUG=
==========

to

==========
Chrome Windows Debugger Extension Skeleton

This is a skeleton of a Chrome Windows Debugger extension that will
provide convenience commands for developers using ntsd, cdb, or windbg.

BUG=

Review-Url: https://codereview.chromium.org/2612193002
Cr-Commit-Position: refs/heads/master@{#443414}
Committed:
https://chromium.googlesource.com/chromium/src/+/fd54b5ccf9c4e391de0bcfe8e852...
==========

[commit-bot: I haz the power, 2017-01-13 00:42:19]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/fd54b5ccf9c4e391de0bcfe8e852...