Use the win10 native avatar button only for AvatarButtonStyle::NATIVE

Use the win10 native avatar button only for AvatarButtonStyle::NATIVE

OpaqueBrowserFrameView creates AvatarButton with AvatarButtonStyle::THEMED,
and this leads to DCHECK_EQ failure at the constructor of AvatarButton.

BUG=723070
Review-Url: https://codereview.chromium.org/2891573002
Cr-Commit-Position: refs/heads/master@{#472443}
Committed: https://chromium.googlesource.com/chromium/src/+/1b08234bf776baa8a8517fc6c0145526055353d0

[Sungmann Cho, 2017-05-16 22:29:58]

sungmann.cho@navercorp.com changed reviewers:
+ emx@chromium.org, pkasting@chromium.org

[Sungmann Cho, 2017-05-16 22:29:58]

Please take a look. Thanks!

[Peter Kasting, 2017-05-17 00:16:41]

What do we want to have happen in the themed-glass-frame case?  Should that be
using the old button or the new one?  I think it's supposed to be using the new
one, in which case the right check isn't UsingSystemTheme() && NATIVE, it's
"whatever we use to check if we're using the glass frame or not".

Preemptive LGTM on that fix.

[Sungmann Cho, 2017-05-17 13:03:49]

On 2017/05/17 00:16:41, Peter Kasting wrote:
> What do we want to have happen in the themed-glass-frame case?  Should that be
> using the old button or the new one?  I think it's supposed to be using the
new
> one, in which case the right check isn't UsingSystemTheme() && NATIVE, it's
> "whatever we use to check if we're using the glass frame or not".
> 
> Preemptive LGTM on that fix.

I agree with your point. Judging from the TODO comment on the if-block, I think
the owner of this feature already has that in mind.

[Sungmann Cho, 2017-05-17 13:04:23]

The CQ bit was checked by sungmann.cho@navercorp.com

[commit-bot: I haz the power, 2017-05-17 13:04:49]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-17 13:52:11]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1495026263884700, "parent_rev":
"5e7860a0564bb5d86d6b9d98022cdadd3f190d54", "commit_rev":
"1b08234bf776baa8a8517fc6c0145526055353d0"}

[commit-bot: I haz the power, 2017-05-17 13:52:21]

Description was changed from

==========
Use the win10 native avatar button only for AvatarButtonStyle::NATIVE

OpaqueBrowserFrameView creates AvatarButton with AvatarButtonStyle::THEMED,
and this leads to DCHECK_EQ failure at the constructor of AvatarButton.

BUG=723070
==========

to

==========
Use the win10 native avatar button only for AvatarButtonStyle::NATIVE

OpaqueBrowserFrameView creates AvatarButton with AvatarButtonStyle::THEMED,
and this leads to DCHECK_EQ failure at the constructor of AvatarButton.

BUG=723070

Review-Url: https://codereview.chromium.org/2891573002
Cr-Commit-Position: refs/heads/master@{#472443}
Committed:
https://chromium.googlesource.com/chromium/src/+/1b08234bf776baa8a8517fc6c014...
==========

[commit-bot: I haz the power, 2017-05-17 13:52:22]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/1b08234bf776baa8a8517fc6c014...

[Peter Kasting, 2017-05-17 18:25:10]

On 2017/05/17 13:03:49, Sungmann Cho wrote:
> On 2017/05/17 00:16:41, Peter Kasting wrote:
> > What do we want to have happen in the themed-glass-frame case?  Should that
be
> > using the old button or the new one?  I think it's supposed to be using the
> new
> > one, in which case the right check isn't UsingSystemTheme() && NATIVE, it's
> > "whatever we use to check if we're using the glass frame or not".
> > 
> > Preemptive LGTM on that fix.
> 
> I agree with your point. Judging from the TODO comment on the if-block, I
think
> the owner of this feature already has that in mind.

The TODO was supposed to be about other versions of Windows and other OSes -- I
don't think this is a case it was trying to cover.  I was trying to say that
that code as-written didn't actually express the particular narrow intent the
original author had, aside from the TODO.

Note also that I had LGTMed the alternate fix I suggested, but not this fix. 
I'm not going to revert (I'm happy to have the DCHECK not fire), but if you can
try and fix as proposed I'd appreciate it!

[Sungmann Cho, 2017-05-17 21:13:28]

> Note also that I had LGTMed the alternate fix I suggested, but not this fix. 
> I'm not going to revert (I'm happy to have the DCHECK not fire), but if you
can
> try and fix as proposed I'd appreciate it!

Oops, I completely misunderstood your comment! I'll create a new CL ASAP.