Add Darwin thread.h implementation.

Add Darwin thread.h implementation.

Due to quirks of the Cocoa runtime several platform-specific fixes were
in place. See the deleted files maccocoathreadhelper and
scoped_autorelease_pool for examples. There is no way to do a stack-based
RAII autoreleasepool that is compatible with ARC, and autoreleasepool
blocks can't be used with c++. The solution was to separate out the
implementation of some methods in thread.h to an ObjC++ file for Darwin
platforms, allowing us to get rid of the helper classes and enable ARC
everywhere.

BUG=webrtc:6412
Review-Url: https://codereview.webrtc.org/2784483002
Cr-Commit-Position: refs/heads/master@{#17436}
Committed: https://chromium.googlesource.com/external/webrtc/+/61abe1582915fb25fe3f231d7d2c9a174c0e3e6b

[kthelgason, 2017-03-28 13:39:43]

kthelgason@webrtc.org changed reviewers:
+ magjed@webrtc.org, tommi@webrtc.org

[kthelgason, 2017-03-28 13:40:46]

PTAL :)

https://codereview.webrtc.org/2784483002/diff/1/webrtc/BUILD.gn
File webrtc/BUILD.gn (left):

https://codereview.webrtc.org/2784483002/diff/1/webrtc/BUILD.gn#oldcode36
webrtc/BUILD.gn:36: "NO_MAIN_THREAD_WRAPPING",
This define is also in webrtc/base/BUILD.gn, closer to where it's used. See
TODO.

[kthelgason, 2017-03-28 13:44:34]

The CQ bit was checked by kthelgason@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 13:44:46]

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

[tommi, 2017-03-28 13:59:01]

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/BUILD.gn
File webrtc/base/BUILD.gn (right):

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/BUILD.gn#newcode547
webrtc/base/BUILD.gn:547: "cocoathread.mm",
I wonder if we should call this file thread_cocoa.mm to keep in line with the
thread_<platform>.<ext> convention. I often use it when working in these files
and think that if someone else is doing the same, it can be good for this file
to pop up alongside the others.

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/thread.h
File webrtc/base/thread.h (right):

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/thread.h#newcode94
webrtc/base/thread.h:94: struct ThreadInit {
can we move this declaration into the source file that needs it? Seems like an
implementation detail.

[commit-bot: I haz the power, 2017-03-28 14:16:50]

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

[commit-bot: I haz the power, 2017-03-28 14:16:51]

Dry run: This issue passed the CQ dry run.

[kthelgason, 2017-03-28 14:29:59]

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/BUILD.gn
File webrtc/base/BUILD.gn (right):

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/BUILD.gn#newcode547
webrtc/base/BUILD.gn:547: "cocoathread.mm",
On 2017/03/28 13:59:01, tommi (webrtc) wrote:
> I wonder if we should call this file thread_cocoa.mm to keep in line with the
> thread_<platform>.<ext> convention. I often use it when working in these files
> and think that if someone else is doing the same, it can be good for this file
> to pop up alongside the others.

Done.

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/thread.h
File webrtc/base/thread.h (right):

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/thread.h#newcode94
webrtc/base/thread.h:94: struct ThreadInit {
On 2017/03/28 13:59:01, tommi (webrtc) wrote:
> can we move this declaration into the source file that needs it? Seems like an
> implementation detail.

I moved it here as it's used in both thread.cc and thread_darwin.mm, I didn't
want to duplicate it in both places. I agree that it's an implementation detail,
so if you want to move it I will.

[tommi, 2017-03-28 14:38:56]

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/thread.h
File webrtc/base/thread.h (right):

https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/thread.h#newcode94
webrtc/base/thread.h:94: struct ThreadInit {
On 2017/03/28 14:29:58, kthelgason wrote:
> On 2017/03/28 13:59:01, tommi (webrtc) wrote:
> > can we move this declaration into the source file that needs it? Seems like
an
> > implementation detail.
> 
> I moved it here as it's used in both thread.cc and thread_darwin.mm, I didn't
> want to duplicate it in both places. I agree that it's an implementation
detail,
> so if you want to move it I will.

ah, can it be a private struct in Thread?

[magjed_webrtc, 2017-03-28 14:41:15]

https://codereview.webrtc.org/2784483002/diff/20001/webrtc/base/thread.cc
File webrtc/base/thread.cc (right):

https://codereview.webrtc.org/2784483002/diff/20001/webrtc/base/thread.cc#new...
webrtc/base/thread.cc:460: #if !defined(WEBRTC_MAC)
Maybe add a comment that the implementation of ProcessMessages is in
thread_darwin.mm for Mac, I was a bit confused by this.

https://codereview.webrtc.org/2784483002/diff/20001/webrtc/base/thread_darwin.mm
File webrtc/base/thread_darwin.mm (right):

https://codereview.webrtc.org/2784483002/diff/20001/webrtc/base/thread_darwin...
webrtc/base/thread_darwin.mm:16: 
Maybe add a comment for this file that it contains darwin specific
implementations of some rtc::Thread methods.

[kthelgason, 2017-03-28 14:50:21]

Thanks for the good comments. Fixed.

https://codereview.webrtc.org/2784483002/diff/20001/webrtc/base/thread.cc
File webrtc/base/thread.cc (right):

https://codereview.webrtc.org/2784483002/diff/20001/webrtc/base/thread.cc#new...
webrtc/base/thread.cc:460: #if !defined(WEBRTC_MAC)
On 2017/03/28 14:41:15, magjed_webrtc wrote:
> Maybe add a comment that the implementation of ProcessMessages is in
> thread_darwin.mm for Mac, I was a bit confused by this.

Done.

https://codereview.webrtc.org/2784483002/diff/20001/webrtc/base/thread_darwin.mm
File webrtc/base/thread_darwin.mm (right):

https://codereview.webrtc.org/2784483002/diff/20001/webrtc/base/thread_darwin...
webrtc/base/thread_darwin.mm:16: 
On 2017/03/28 14:41:15, magjed_webrtc wrote:
> Maybe add a comment for this file that it contains darwin specific
> implementations of some rtc::Thread methods.

Done.

[kthelgason, 2017-03-28 16:07:21]

On 2017/03/28 14:38:56, tommi (webrtc) wrote:
> https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/thread.h
> File webrtc/base/thread.h (right):
> 
> https://codereview.webrtc.org/2784483002/diff/1/webrtc/base/thread.h#newcode94
> webrtc/base/thread.h:94: struct ThreadInit {
> On 2017/03/28 14:29:58, kthelgason wrote:
> > On 2017/03/28 13:59:01, tommi (webrtc) wrote:
> > > can we move this declaration into the source file that needs it? Seems
like
> an
> > > implementation detail.
> > 
> > I moved it here as it's used in both thread.cc and thread_darwin.mm, I
didn't
> > want to duplicate it in both places. I agree that it's an implementation
> detail,
> > so if you want to move it I will.
> 
> ah, can it be a private struct in Thread?

Done.

[tommi, 2017-03-28 22:29:17]

lgtm

[magjed_webrtc, 2017-03-29 08:16:57]

lgtm

[kthelgason, 2017-03-29 08:29:20]

The CQ bit was checked by kthelgason@webrtc.org

[commit-bot: I haz the power, 2017-03-29 08:29:31]

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

[commit-bot: I haz the power, 2017-03-29 09:32:37]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490776160469510,
"parent_rev": "0f0a849cba51ae68e508c04f4abac9e36fbbeb05", "commit_rev":
"61abe1582915fb25fe3f231d7d2c9a174c0e3e6b"}

[commit-bot: I haz the power, 2017-03-29 09:32:39]

Description was changed from

==========
Add Darwin thread.h implementation.

Due to quirks of the Cocoa runtime several platform-specific fixes were
in place. See the deleted files maccocoathreadhelper and
scoped_autorelease_pool for examples. There is no way to do a stack-based
RAII autoreleasepool that is compatible with ARC, and autoreleasepool
blocks can't be used with c++. The solution was to separate out the
implementation of some methods in thread.h to an ObjC++ file for Darwin
platforms, allowing us to get rid of the helper classes and enable ARC
everywhere.

BUG=webrtc:6412
==========

to

==========
Add Darwin thread.h implementation.

Due to quirks of the Cocoa runtime several platform-specific fixes were
in place. See the deleted files maccocoathreadhelper and
scoped_autorelease_pool for examples. There is no way to do a stack-based
RAII autoreleasepool that is compatible with ARC, and autoreleasepool
blocks can't be used with c++. The solution was to separate out the
implementation of some methods in thread.h to an ObjC++ file for Darwin
platforms, allowing us to get rid of the helper classes and enable ARC
everywhere.

BUG=webrtc:6412

Review-Url: https://codereview.webrtc.org/2784483002
Cr-Commit-Position: refs/heads/master@{#17436}
Committed:
https://chromium.googlesource.com/external/webrtc/+/61abe1582915fb25fe3f231d7...
==========

[commit-bot: I haz the power, 2017-03-29 09:32:40]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/61abe1582915fb25fe3f231d7...