Use a singleton instead of a global for MojoWaitManyState

Use a singleton instead of a global for MojoWaitManyState

R=abarth@chromium.org, jamesr@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/9aaee9d23fa645ff87eddc35078f2d80124f0bf0

[jsimmons1, 2015-10-09 23:36:01]

jsimmons@google.com changed reviewers:
+ abarth@chromium.org, jamesr@chromium.org

[abarth-chromium, 2015-10-09 23:41:36]

LGTM

[jamesr, 2015-10-09 23:42:32]

https://codereview.chromium.org/1395043003/diff/1/mojo/dart/embedder/mojo_nat...
File mojo/dart/embedder/mojo_natives.cc (right):

https://codereview.chromium.org/1395043003/diff/1/mojo/dart/embedder/mojo_nat...
mojo/dart/embedder/mojo_natives.cc:739: return
Singleton<MojoWaitManyState>::get();
Singleton<> here is providing thread-safe initialization, which you don't need
as this code isn't threadsafe anyway. do this instead:

  static MojoWaitManyState* state = new MojoWaitManyState;
  return state;

and remove the //base/memory/singleton.h dependency

https://codereview.chromium.org/1395043003/diff/1/mojo/dart/embedder/mojo_nat...
mojo/dart/embedder/mojo_natives.cc:746: MojoWaitManyState*
handle_watcher_wait_state =
Make this a reference and you won't have to change the following lines. it can
never be null

[abarth-chromium, 2015-10-09 23:45:47]

https://codereview.chromium.org/1395043003/diff/1/mojo/dart/embedder/mojo_nat...
File mojo/dart/embedder/mojo_natives.cc (right):

https://codereview.chromium.org/1395043003/diff/1/mojo/dart/embedder/mojo_nat...
mojo/dart/embedder/mojo_natives.cc:746: MojoWaitManyState*
handle_watcher_wait_state =
On 2015/10/09 at 23:42:32, jamesr wrote:
> Make this a reference and you won't have to change the following lines. it can
never be null

If you make this a reference, please add DISALLOW_COPY_AND_ASSIGN to
MojoWaitManyState.  (You might want to do that anyway.)

[jsimmons1, 2015-10-09 23:50:47]

https://codereview.chromium.org/1395043003/diff/1/mojo/dart/embedder/mojo_nat...
File mojo/dart/embedder/mojo_natives.cc (right):

https://codereview.chromium.org/1395043003/diff/1/mojo/dart/embedder/mojo_nat...
mojo/dart/embedder/mojo_natives.cc:739: return
Singleton<MojoWaitManyState>::get();
On 2015/10/09 23:42:32, jamesr wrote:
> Singleton<> here is providing thread-safe initialization, which you don't need
> as this code isn't threadsafe anyway. do this instead:
> 
>   static MojoWaitManyState* state = new MojoWaitManyState;
>   return state;
> 
> and remove the //base/memory/singleton.h dependency

Done.

https://codereview.chromium.org/1395043003/diff/1/mojo/dart/embedder/mojo_nat...
mojo/dart/embedder/mojo_natives.cc:746: MojoWaitManyState*
handle_watcher_wait_state =
On 2015/10/09 23:42:32, jamesr wrote:
> Make this a reference and you won't have to change the following lines. it can
> never be null

Done.

[jamesr, 2015-10-10 00:02:11]

I'm not seeing a new patch set here - run 'git cl upload' to put one up.

[jamesr, 2015-10-10 00:04:11]

I see it now, lgtm

[jsimmons1, 2015-10-10 00:19:12]

Committed patchset #2 (id:20001) manually as
9aaee9d23fa645ff87eddc35078f2d80124f0bf0 (presubmit successful).