base/threading: minor cleanups to remove friend class

base/threading: minor cleanups to remove friend class

Do not touch PlatformThreadHandle internal members directly, but use
existing getter methods instead.

BUG=495097
Committed: https://crrev.com/58268bc285589dc022db434cd4da2a67b862ec3b
Cr-Commit-Position: refs/heads/master@{#332577}

[Takashi Toyoshima, 2015-06-02 08:02:53]

toyoshim@chromium.org changed reviewers:
+ gab@chromium.org

[Takashi Toyoshima, 2015-06-02 08:57:09]

Can you take a look?

https://codereview.chromium.org/1164713007/diff/40001/base/threading/platform...
File base/threading/platform_thread_win.cc (right):

https://codereview.chromium.org/1164713007/diff/40001/base/threading/platform...
base/threading/platform_thread_win.cc:227: CHECK(false);
No one work on this bug now, and the bug 127931 was already closed.
But, when I checked crash reports, there are still many reports that hit this
CHECK at line 227.

As far as I looked, the callstacks say many things more than the thread handle
value. So, I think removing debug::Alias for the thread handle to drop the
friend declaration is reasonable.

[gab, 2015-06-02 14:15:40]

gab@chromium.org changed reviewers:
+ rvargas@chromium.org

[gab, 2015-06-02 14:15:41]

lgtm -- TBR rvargas (see below).

https://codereview.chromium.org/1164713007/diff/40001/base/threading/platform...
File base/threading/platform_thread_win.cc (right):

https://codereview.chromium.org/1164713007/diff/40001/base/threading/platform...
base/threading/platform_thread_win.cc:227: CHECK(false);
On 2015/06/02 08:57:08, Takashi Toyoshima (chromium) wrote:
> No one work on this bug now, and the bug 127931 was already closed.
> But, when I checked crash reports, there are still many reports that hit this
> CHECK at line 227.
> 
> As far as I looked, the callstacks say many things more than the thread handle
> value. So, I think removing debug::Alias for the thread handle to drop the
> friend declaration is reasonable.

I think this should be okay since |thread_handle| is on the stack as a parameter
anyways (don't see why it wouldn't be in a crash even if not explicitly
aliased).

TBR rvargas

[gab, 2015-06-02 14:17:08]

On 2015/06/02 14:15:41, gab wrote:
> lgtm -- TBR rvargas (see below).
> 
>
https://codereview.chromium.org/1164713007/diff/40001/base/threading/platform...
> File base/threading/platform_thread_win.cc (right):
> 
>
https://codereview.chromium.org/1164713007/diff/40001/base/threading/platform...
> base/threading/platform_thread_win.cc:227: CHECK(false);
> On 2015/06/02 08:57:08, Takashi Toyoshima (chromium) wrote:
> > No one work on this bug now, and the bug 127931 was already closed.
> > But, when I checked crash reports, there are still many reports that hit
this
> > CHECK at line 227.
> > 
> > As far as I looked, the callstacks say many things more than the thread
handle
> > value. So, I think removing debug::Alias for the thread handle to drop the
> > friend declaration is reasonable.
> 
> I think this should be okay since |thread_handle| is on the stack as a
parameter
> anyways (don't see why it wouldn't be in a crash even if not explicitly
> aliased).
> 
> TBR rvargas

PS: I pinged on http://crbug.com/127931 to see whether this instrumentation was
still useful, for now go ahead and strip this part, we can find another solution
later if it's still needed.

[Takashi Toyoshima, 2015-06-03 01:41:15]

Thank you gab.
rvargas looks taking a long vacation for some weeks.
I'd ask another person for a stamp to land.

[Takashi Toyoshima, 2015-06-03 01:41:42]

toyoshim@chromium.org changed reviewers:
+ thakis@chromium.org

[Takashi Toyoshima, 2015-06-03 01:42:51]

+Nico

Hi Nico,
This CL already got a review by gab, but can you take another look for OWNERS's
stamp?

[Nico, 2015-06-03 02:30:09]

lgtm

[Takashi Toyoshima, 2015-06-03 03:09:03]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2015-06-03 03:12:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1164713007/40001

[commit-bot: I haz the power, 2015-06-03 07:22:14]

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

[commit-bot: I haz the power, 2015-06-03 07:22:15]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[Takashi Toyoshima, 2015-06-03 07:26:31]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2015-06-03 07:27:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1164713007/40001

[commit-bot: I haz the power, 2015-06-03 08:48:45]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-03 08:49:33]

Patchset 3 (id:??) landed as
https://crrev.com/58268bc285589dc022db434cd4da2a67b862ec3b
Cr-Commit-Position: refs/heads/master@{#332577}