Reenable the --enable-nacl-debug switch

Turn on --enable-nacl-debug.


Reenable the --enable-nacl-debug switch.

This was accidentally turned off in a bigger CL:
  http://codereview.chromium.org/7395024/
See nacl_main.cc and nacl_listener.cc in that CL to compare to this.

BUG= http://code.google.com/p/chromium/issues/detail?id=91425

TEST= manual - w/o this fix, chrome --enable-nacl-debug no longer works, with
this change, I was able to launch Chrome from Visual Studio and set breakpoints
in a nexe.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95253

[mmortensen, 2011-08-02 22:12:29]

Running trybots now, manual test of fix was successful.

[Brad Chen, 2011-08-02 22:56:51]

LGTM

Thanks for fixing this!

[Brad Chen, 2011-08-02 22:57:25]

Remember to use the trybots, including linux_clang.

On Tue, Aug 2, 2011 at 3:12 PM, <mmortensen@google.com> wrote:

> Reviewers: dmichael, Brad Chen, mlinck,
>
> Message:
> Running trybots now, manual test of fix was successful.
>
> Description:
> Reenable the --enable-nacl-debug switch.
>
> This was accidentally turned off in a bigger CL:
>  
http://codereview.chromium.**org/7395024/<http://codereview.chromium.org/7395...
> See nacl_main.cc and nacl_listener.cc in that CL to compare to this.
>
> BUG=
http://code.google.com/p/**chromium/issues/detail?id=**91425<http://code.goog...
>
> TEST= manual - w/o this fix, chrome --enable-nacl-debug no longer works,
> with
> this change, I was able to launch Chrome from Visual Studio and set
> breakpoints
> in a nexe.
>
>
> Please review this at
http://codereview.chromium.**org/7547002/<http://codereview.chromium.org/7547...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/>
>
> Affected files:
>  M     chrome/nacl/nacl_listener.h
>  M     chrome/nacl/nacl_listener.cc
>  M     chrome/nacl/nacl_main.cc
>
>
> Index: chrome/nacl/nacl_listener.cc
> ==============================**==============================**=======
> --- chrome/nacl/nacl_listener.cc        (revision 95033)
> +++ chrome/nacl/nacl_listener.cc        (working copy)
> @@ -74,7 +74,7 @@
>                                     int debug);
>  extern "C" void NaClSetIrtFileDesc(int fd);
>
> -NaClListener::NaClListener() {}
> +NaClListener::NaClListener(**bool debug) : debug_enabled_(debug) {}
>
>  NaClListener::~NaClListener() {}
>
> @@ -131,6 +131,6 @@
>     array[i] = nacl::ToNativeHandle(handles[**i]);
>   }
>   NaClMainForChromium(static_**cast<int>(handles.size()), array.get(),
> -                      false /* debug */);
> +                      debug_enabled_);
>   NOTREACHED();
>  }
> Index: chrome/nacl/nacl_listener.h
> ==============================**==============================**=======
> --- chrome/nacl/nacl_listener.h (revision 95033)
> +++ chrome/nacl/nacl_listener.h (working copy)
> @@ -15,12 +15,13 @@
>  // request to start a NaCl module.
>  class NaClListener : public IPC::Channel::Listener {
>  public:
> -  NaClListener();
> +  explicit NaClListener(bool debug);
>   virtual ~NaClListener();
>   // Listen for a request to launch a NaCl module.
>   void Listen();
>
>  private:
> +  bool debug_enabled_;
>   void OnStartSelLdr(std::vector<**nacl::FileDescriptor> handles,
>                      bool have_irt_file);
>   virtual bool OnMessageReceived(const IPC::Message& msg);
> Index: chrome/nacl/nacl_main.cc
> ==============================**==============================**=======
> --- chrome/nacl/nacl_main.cc    (revision 95033)
> +++ chrome/nacl/nacl_main.cc    (working copy)
> @@ -119,7 +119,8 @@
>   bool sandbox_test_result = platform.RunSandboxTests();
>
>   if (sandbox_test_result) {
> -    NaClListener listener;
> +    bool debug = parsed_command_line.HasSwitch(**
> switches::kEnableNaClDebug);
> +    NaClListener listener(debug);
>     listener.Listen();
>   } else {
>     // This indirectly prevents the test-harness-success-cookie from being
> set,
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[Brad Chen, 2011-08-02 22:57:27]

Remember to use the trybots, including linux_clang.

On Tue, Aug 2, 2011 at 3:12 PM, <mmortensen@google.com> wrote:

> Reviewers: dmichael, Brad Chen, mlinck,
>
> Message:
> Running trybots now, manual test of fix was successful.
>
> Description:
> Reenable the --enable-nacl-debug switch.
>
> This was accidentally turned off in a bigger CL:
>  
http://codereview.chromium.**org/7395024/<http://codereview.chromium.org/7395...
> See nacl_main.cc and nacl_listener.cc in that CL to compare to this.
>
> BUG=
http://code.google.com/p/**chromium/issues/detail?id=**91425<http://code.goog...
>
> TEST= manual - w/o this fix, chrome --enable-nacl-debug no longer works,
> with
> this change, I was able to launch Chrome from Visual Studio and set
> breakpoints
> in a nexe.
>
>
> Please review this at
http://codereview.chromium.**org/7547002/<http://codereview.chromium.org/7547...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/>
>
> Affected files:
>  M     chrome/nacl/nacl_listener.h
>  M     chrome/nacl/nacl_listener.cc
>  M     chrome/nacl/nacl_main.cc
>
>
> Index: chrome/nacl/nacl_listener.cc
> ==============================**==============================**=======
> --- chrome/nacl/nacl_listener.cc        (revision 95033)
> +++ chrome/nacl/nacl_listener.cc        (working copy)
> @@ -74,7 +74,7 @@
>                                     int debug);
>  extern "C" void NaClSetIrtFileDesc(int fd);
>
> -NaClListener::NaClListener() {}
> +NaClListener::NaClListener(**bool debug) : debug_enabled_(debug) {}
>
>  NaClListener::~NaClListener() {}
>
> @@ -131,6 +131,6 @@
>     array[i] = nacl::ToNativeHandle(handles[**i]);
>   }
>   NaClMainForChromium(static_**cast<int>(handles.size()), array.get(),
> -                      false /* debug */);
> +                      debug_enabled_);
>   NOTREACHED();
>  }
> Index: chrome/nacl/nacl_listener.h
> ==============================**==============================**=======
> --- chrome/nacl/nacl_listener.h (revision 95033)
> +++ chrome/nacl/nacl_listener.h (working copy)
> @@ -15,12 +15,13 @@
>  // request to start a NaCl module.
>  class NaClListener : public IPC::Channel::Listener {
>  public:
> -  NaClListener();
> +  explicit NaClListener(bool debug);
>   virtual ~NaClListener();
>   // Listen for a request to launch a NaCl module.
>   void Listen();
>
>  private:
> +  bool debug_enabled_;
>   void OnStartSelLdr(std::vector<**nacl::FileDescriptor> handles,
>                      bool have_irt_file);
>   virtual bool OnMessageReceived(const IPC::Message& msg);
> Index: chrome/nacl/nacl_main.cc
> ==============================**==============================**=======
> --- chrome/nacl/nacl_main.cc    (revision 95033)
> +++ chrome/nacl/nacl_main.cc    (working copy)
> @@ -119,7 +119,8 @@
>   bool sandbox_test_result = platform.RunSandboxTests();
>
>   if (sandbox_test_result) {
> -    NaClListener listener;
> +    bool debug = parsed_command_line.HasSwitch(**
> switches::kEnableNaClDebug);
> +    NaClListener listener(debug);
>     listener.Listen();
>   } else {
>     // This indirectly prevents the test-harness-success-cookie from being
> set,
>
>
>

[dmichael (off chromium), 2011-08-03 14:19:51]

Can you add a BUG= and TEST= to the CL? Even if they're 'None' (though a BUG
might be a good idea).

Also, can you reference the original CL where the flag was disabled?

o/w LGTM

[mlinck, 2011-08-03 14:23:12]

LGTM

[mmortensen, 2011-08-03 21:57:03]

On 2011/08/03 14:26:16, mmortensen wrote:

Committed as 95253