Say "noto" goto

Say "noto" goto.

Also, fix DCHECK due to incorrect cleanup when app_library fails to load.

R=ben@chromium.org, darin@chromium.org

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

[Ben Goodger (Google), 2013-11-01 21:38:12]

cool!

lgtm

[abarth-chromium, 2013-11-01 21:43:49]

\o/

[darin (slow to review), 2013-11-01 21:48:52]

https://codereview.chromium.org/56553002/diff/1/mojo/public/system/core.h
File mojo/public/system/core.h (right):

https://codereview.chromium.org/56553002/diff/1/mojo/public/system/core.h#new...
mojo/public/system/core.h:313: // Closes handle_ when deleted.
nit: how about breaking this out into its own header file. I suspect we may have
more utilities like this in the future.

(We may also want to embolden this class with move semantic goodness as in
scoped_ptr.h, but that can be for another day.)

[Aaron Boodman, 2013-11-01 21:54:37]

On 2013/11/01 21:48:52, darin wrote:
> https://codereview.chromium.org/56553002/diff/1/mojo/public/system/core.h
> File mojo/public/system/core.h (right):
> 
>
https://codereview.chromium.org/56553002/diff/1/mojo/public/system/core.h#new...
> mojo/public/system/core.h:313: // Closes handle_ when deleted.
> nit: how about breaking this out into its own header file. I suspect we may
have
> more utilities like this in the future.

OK.

> (We may also want to embolden this class with move semantic goodness as in
> scoped_ptr.h, but that can be for another day.)

Yeah, that's what I was alluding to in the class header. I'm sure there's lots
of fanciness we will want, but I figured getting this in place in a simple form
was important so that callsites start using it rather than bare handles. Then
incrementally improving it helps globally.

[Aaron Boodman, 2013-11-01 22:34:02]

darin, ptal

[darin (slow to review), 2013-11-01 22:39:42]

https://codereview.chromium.org/56553002/diff/120001/mojo/public/system/scope...
File mojo/public/system/scoped_handle.h (right):

https://codereview.chromium.org/56553002/diff/120001/mojo/public/system/scope...
mojo/public/system/scoped_handle.h:9: #ifdef __cplusplus
How about emitting a #error if this header file is not included by a C++
program?

https://codereview.chromium.org/56553002/diff/120001/mojo/public/system/scope...
mojo/public/system/scoped_handle.h:18: class MOJO_SYSTEM_EXPORT ScopedHandle {
I think there was a goal to minimize the exports of mojo_system to just the core
API, and the exports should just be extern "C" since C++ doesn't specify how
symbols get mangled. We don't want to make our ABI specific to a particular C++
compiler.

Either ScopedHandle should be implemented inline, or we should punt this out of
core/ and create a static helper library for this class. That might be a better
option. Consider:

  mojo/public/utility/

That directory could have various classes like ScopedHandle and any other
utilities that make it easier to work with the core API.

[Aaron Boodman, 2013-11-01 22:46:20]

Right, I remember now. Can we assume that all clients of this library will be
C++?

[Aaron Boodman, 2013-11-01 23:00:20]

Darin, ptal.

[darin (slow to review), 2013-11-01 23:04:52]

https://codereview.chromium.org/56553002/diff/180001/mojo/public/utility/scop...
File mojo/public/utility/scoped_handle.h (right):

https://codereview.chromium.org/56553002/diff/180001/mojo/public/utility/scop...
mojo/public/utility/scoped_handle.h:15: class MOJO_SYSTEM_EXPORT ScopedHandle {
nit: no need for the export. this is a static library.

[darin (slow to review), 2013-11-01 23:05:13]

https://codereview.chromium.org/56553002/diff/180001/mojo/public/utility/scop...
File mojo/public/utility/scoped_handle.h (right):

https://codereview.chromium.org/56553002/diff/180001/mojo/public/utility/scop...
mojo/public/utility/scoped_handle.h:9: #include
"mojo/public/system/system_export.h"
LGTM w/ this line and the MOJO_SYSTEM_EXPORT removed.

[commit-bot: I haz the power, 2013-11-01 23:25:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/56553002/230001

[commit-bot: I haz the power, 2013-11-01 23:59:38]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-04 06:05:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/56553002/410001

[commit-bot: I haz the power, 2013-11-04 07:54:37]

Retried try job too often on win_rel for step(s) content_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Aaron Boodman, 2013-11-04 19:24:24]

Committed patchset #6 manually as r232756 (presubmit successful).