Allow multiple NaCl modules to be debugged.

Allow multiple NaCl modules to be debugged.

Assigning the first NaCl module to load to port 4014, allowing other to be
dynamic, using a code path currently used for testing.

Expose the port used by each module in the Task Manager UI to make it available
to users debugging NaCl modules.

Follow on changes will add extension api fields to make debuggable modules
discoverable to a web based debugger.

NOTE: The debug stub port on windows is opened at a different layer (inside nacl, only availabled with --disable-sandbox). This will be exposed in a follow on change.

Reviewers:
yoshiki@chromium.org (for task_manager)
pkasting@chromium.org (for browser/ui)
brettw@chromium.org (for content)
noelallen@chromium.org (for nacl)

BUG=328714
TEST=trybots
R=yoshiki@chromium.org,pkasting@chromium.org,brettw@chromium.org,noelallen@chromium.org

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

[Peter Kasting, 2013-12-17 23:12:29]

LGTM

https://codereview.chromium.org/102073008/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/task_manager_mac.mm (right):

https://codereview.chromium.org/102073008/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/task_manager_mac.mm:66: arraysize("100") * kCharWidth,
-1 },
Shouldn't this be 5 chars instead of 3?

[bradn, 2013-12-20 00:35:37]

PTAL

Ping other reviews?

https://codereview.chromium.org/102073008/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/task_manager_mac.mm (right):

https://codereview.chromium.org/102073008/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/task_manager_mac.mm:66: arraysize("100") * kCharWidth,
-1 },
On 2013/12/17 23:12:29, Peter Kasting wrote:
> Shouldn't this be 5 chars instead of 3?

Done.

[noelallen_use_chromium, 2013-12-20 01:00:27]

LGTM post changes.

https://codereview.chromium.org/102073008/diff/60001/components/nacl/browser/...
File components/nacl/browser/nacl_process_host.cc (right):

https://codereview.chromium.org/102073008/diff/60001/components/nacl/browser/...
components/nacl/browser/nacl_process_host.cc:673: // We allocate currently
unused TCP port for debug stub tests. The port
Update comment.

We always try to allocate the default port.  If it fails, we then allocate any
port.  And notify the test system if it's running under test
(HasGdbDebutStubPortListener).

https://codereview.chromium.org/102073008/diff/60001/components/nacl/browser/...
components/nacl/browser/nacl_process_host.cc:687: LOG(ERROR) << "failed to open
socket for debug stub";
print port number?

[bradn, 2013-12-20 01:25:34]

PTAL other OWNERS

https://codereview.chromium.org/102073008/diff/60001/components/nacl/browser/...
File components/nacl/browser/nacl_process_host.cc (right):

https://codereview.chromium.org/102073008/diff/60001/components/nacl/browser/...
components/nacl/browser/nacl_process_host.cc:673: // We allocate currently
unused TCP port for debug stub tests. The port
On 2013/12/20 01:00:28, noelallen wrote:
> Update comment.
> 
> We always try to allocate the default port.  If it fails, we then allocate any
> port.  And notify the test system if it's running under test
> (HasGdbDebutStubPortListener).

Done.

https://codereview.chromium.org/102073008/diff/60001/components/nacl/browser/...
components/nacl/browser/nacl_process_host.cc:687: LOG(ERROR) << "failed to open
socket for debug stub";
On 2013/12/20 01:00:28, noelallen wrote:
> print port number?

Done.

[bradn, 2014-01-07 19:43:37]

Ping OWNERS:
yoshiki@chromium.org (for task_manager) 
brettw@chromium.org (for content)

[brettw, 2014-01-08 06:06:10]

content lgtm

[bradn, 2014-01-08 18:36:23]

ping yoshiki

[yoshiki, 2014-01-09 02:18:26]

lgtm

[commit-bot: I haz the power, 2014-01-09 21:34:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bradnelson@google.com/102073008/550001

[commit-bot: I haz the power, 2014-01-09 21:58:03]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[noelallen1, 2014-01-09 22:19:01]

lgtm

[commit-bot: I haz the power, 2014-01-09 22:21:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bradnelson@google.com/102073008/550001

[commit-bot: I haz the power, 2014-01-10 02:13:15]

Change committed as 244041

[jam, 2014-05-15 03:50:12]

i just saw this cl. content doesn't know anything about nacl, so this shouldn't
have changed content.

is this still needed? if not, then please remove. otherwise, please find a way
so that chrome and components talk directly without going through content.

[chromium-reviews, 2014-05-15 17:17:09]

Stil used, looking into routing round.
I may swing by to ask advice.
-BradN


On Wed, May 14, 2014 at 8:50 PM, <jam@chromium.org> wrote:

> i just saw this cl. content doesn't know anything about nacl, so this
> shouldn't
> have changed content.
>
> is this still needed? if not, then please remove. otherwise, please find a
> way
> so that chrome and components talk directly without going through content.
>
> https://codereview.chromium.org/102073008/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[bradn, 2014-05-15 23:14:33]

Addressing concerns about content in:
https://codereview.chromium.org/286993006/