Reland 097f9cde453ea57eb4aa037f44add782391c5eb9

Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

This is a reland of 097f9cde453ea57eb4aa037f44add782391c5eb9 with fix
for asan failure.

BUG=711183, 693282
TBR=stevenjb@chromium.org,tdanderson@chromium.org

Review-Url: https://codereview.chromium.org/2838273002
Cr-Commit-Position: refs/heads/master@{#468157}
Committed: https://chromium.googlesource.com/chromium/src/+/56fc9d639f67506841a0cb90a421a869e5107eea

[Evan Stade, 2017-04-25 22:09:32]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 22:09:59]

Dry run: CQ is trying da patch.

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

[Evan Stade, 2017-04-25 22:10:11]

Description was changed from

==========
Revert "Revert of Introduce a type of View background that stays in sync with
its host (patchset #10 id:180001 of https://codereview.chromium.org/2816193002/
)"

This reverts commit 1cf2905163bc2e0ec4ae52c533e751f4de7cde20.

BUG=
==========

to

==========
Reland 097f9cde453ea57eb4aa037f44add782391c5eb9:
Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

BUG=711183, 693282
TBR=stevenjb@chromium.org
==========

[commit-bot: I haz the power, 2017-04-25 23:26:10]

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

[commit-bot: I haz the power, 2017-04-25 23:26:11]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2017-04-25 23:28:45]

Description was changed from

==========
Reland 097f9cde453ea57eb4aa037f44add782391c5eb9:
Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

BUG=711183, 693282
TBR=stevenjb@chromium.org
==========

to

==========
Reland 097f9cde453ea57eb4aa037f44add782391c5eb9:
Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

BUG=711183, 693282
TBR=stevenjb@chromium.org,tdanderson@chromium.org
==========

[Evan Stade, 2017-04-25 23:28:45]

estade@chromium.org changed reviewers:
+ msw@chromium.org

[Evan Stade, 2017-04-26 14:32:07]

msw, PTAL. PS1 is original, PS2 contains the fix.

[Peter Beverloo, 2017-04-26 14:42:47]

On 2017/04/26 14:32:07, Evan Stade wrote:
> msw, PTAL. PS1 is original, PS2 contains the fix.

Random drive-by: please give this a sensible title. Only "Reland
097f9cde453ea57eb4aa037f44add782391c5eb9" would show up in `git log --oneline`,
which isn't useful.

[Evan Stade, 2017-04-26 15:09:25]

On 2017/04/26 14:42:47, Peter Beverloo wrote:
> On 2017/04/26 14:32:07, Evan Stade wrote:
> > msw, PTAL. PS1 is original, PS2 contains the fix.
> 
> Random drive-by: please give this a sensible title. Only "Reland
> 097f9cde453ea57eb4aa037f44add782391c5eb9" would show up in `git log
--oneline`,
> which isn't useful.

Seems useful to me, but maybe I don't know what task you're judging its
usefulness by. If that's insufficient for the purposes of your log maybe don't
use --oneline?

[msw, 2017-04-26 21:39:47]

On 2017/04/26 15:09:25, Evan Stade wrote:
> On 2017/04/26 14:42:47, Peter Beverloo wrote:
> > On 2017/04/26 14:32:07, Evan Stade wrote:
> > > msw, PTAL. PS1 is original, PS2 contains the fix.
> > 
> > Random drive-by: please give this a sensible title. Only "Reland
> > 097f9cde453ea57eb4aa037f44add782391c5eb9" would show up in `git log
> --oneline`,
> > which isn't useful.
> 
> Seems useful to me, but maybe I don't know what task you're judging its
> usefulness by. If that's insufficient for the purposes of your log maybe don't
> use --oneline?

I agree that "reland <reverted git hash>" is not a useful title; it's akin to
"land <rietveld issue #>" for any other CL. A useful title briefly describes the
purpose of the change (ie. "views: Add themed Background support."), imparting
knowledge to the reader without pointing them elsewhere. Mentioning that this is
an attempt to reland an earlier CL seems more suited to part of the CL
description (ie. "The prior CL <foo> was reverted because of <bar>, this
resolves that defect by <baz>"). Otherwise lgtm.

[Evan Stade, 2017-04-26 21:44:54]

Description was changed from

==========
Reland 097f9cde453ea57eb4aa037f44add782391c5eb9:
Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

BUG=711183, 693282
TBR=stevenjb@chromium.org,tdanderson@chromium.org
==========

to

==========
Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

This is a reland of 097f9cde453ea57eb4aa037f44add782391c5eb9 with fix
for asan failure.

BUG=711183, 693282
TBR=stevenjb@chromium.org,tdanderson@chromium.org
==========

[Evan Stade, 2017-04-26 21:49:01]

On 2017/04/26 21:39:47, msw wrote:
> On 2017/04/26 15:09:25, Evan Stade wrote:
> > On 2017/04/26 14:42:47, Peter Beverloo wrote:
> > > On 2017/04/26 14:32:07, Evan Stade wrote:
> > > > msw, PTAL. PS1 is original, PS2 contains the fix.
> > > 
> > > Random drive-by: please give this a sensible title. Only "Reland
> > > 097f9cde453ea57eb4aa037f44add782391c5eb9" would show up in `git log
> > --oneline`,
> > > which isn't useful.
> > 
> > Seems useful to me, but maybe I don't know what task you're judging its
> > usefulness by. If that's insufficient for the purposes of your log maybe
don't
> > use --oneline?
> 
> I agree that "reland <reverted git hash>" is not a useful title; it's akin to
> "land <rietveld issue #>" for any other CL. A useful title briefly describes
the
> purpose of the change (ie. "views: Add themed Background support."), imparting
> knowledge to the reader without pointing them elsewhere. Mentioning that this
is
> an attempt to reland an earlier CL seems more suited to part of the CL
> description (ie. "The prior CL <foo> was reverted because of <bar>, this
> resolves that defect by <baz>"). Otherwise lgtm.

I've adopted your suggestion but in some places where I am reading CL
descriptions, that seems like burying the lede. For example, on the bug tracker
it's going to be difficult to tell the difference between a land and re-land if
I have to dig deep into the description to figure out what the difference is.
When you revert, "revert" is the first word in the description, which I think is
best.

[Evan Stade, 2017-04-26 21:49:25]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-04-26 21:50:41]

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-04-26 22:04:26]

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

[commit-bot: I haz the power, 2017-04-26 22:04:28]

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

[msw, 2017-04-26 23:17:43]

On 2017/04/26 21:49:01, Evan Stade wrote:
> On 2017/04/26 21:39:47, msw wrote:
> > On 2017/04/26 15:09:25, Evan Stade wrote:
> > > On 2017/04/26 14:42:47, Peter Beverloo wrote:
> > > > On 2017/04/26 14:32:07, Evan Stade wrote:
> > > > > msw, PTAL. PS1 is original, PS2 contains the fix.
> > > > 
> > > > Random drive-by: please give this a sensible title. Only "Reland
> > > > 097f9cde453ea57eb4aa037f44add782391c5eb9" would show up in `git log
> > > --oneline`,
> > > > which isn't useful.
> > > 
> > > Seems useful to me, but maybe I don't know what task you're judging its
> > > usefulness by. If that's insufficient for the purposes of your log maybe
> don't
> > > use --oneline?
> > 
> > I agree that "reland <reverted git hash>" is not a useful title; it's akin
to
> > "land <rietveld issue #>" for any other CL. A useful title briefly describes
> the
> > purpose of the change (ie. "views: Add themed Background support."),
imparting
> > knowledge to the reader without pointing them elsewhere. Mentioning that
this
> is
> > an attempt to reland an earlier CL seems more suited to part of the CL
> > description (ie. "The prior CL <foo> was reverted because of <bar>, this
> > resolves that defect by <baz>"). Otherwise lgtm.
> 
> I've adopted your suggestion but in some places where I am reading CL
> descriptions, that seems like burying the lede. For example, on the bug
tracker
> it's going to be difficult to tell the difference between a land and re-land
if
> I have to dig deep into the description to figure out what the difference is.
> When you revert, "revert" is the first word in the description, which I think
is
> best.

You haven't changed the issue title here afaict, but maybe that's different than
the CL's title? (perhaps the git change title actually comes from the first line
of the CL description above? I never actually make those different.) Yeah, feel
free to pre-pend "Reland:" (or similar) to the front of CL titles when
re-landing something that was reverted, I usually do. So I'd actually use a
title like "Reland: views: Add themed Background support."

[msw, 2017-04-26 23:18:10]

On 2017/04/26 23:17:43, msw wrote:
> On 2017/04/26 21:49:01, Evan Stade wrote:
> > On 2017/04/26 21:39:47, msw wrote:
> > > On 2017/04/26 15:09:25, Evan Stade wrote:
> > > > On 2017/04/26 14:42:47, Peter Beverloo wrote:
> > > > > On 2017/04/26 14:32:07, Evan Stade wrote:
> > > > > > msw, PTAL. PS1 is original, PS2 contains the fix.
> > > > > 
> > > > > Random drive-by: please give this a sensible title. Only "Reland
> > > > > 097f9cde453ea57eb4aa037f44add782391c5eb9" would show up in `git log
> > > > --oneline`,
> > > > > which isn't useful.
> > > > 
> > > > Seems useful to me, but maybe I don't know what task you're judging its
> > > > usefulness by. If that's insufficient for the purposes of your log maybe
> > don't
> > > > use --oneline?
> > > 
> > > I agree that "reland <reverted git hash>" is not a useful title; it's akin
> to
> > > "land <rietveld issue #>" for any other CL. A useful title briefly
describes
> > the
> > > purpose of the change (ie. "views: Add themed Background support."),
> imparting
> > > knowledge to the reader without pointing them elsewhere. Mentioning that
> this
> > is
> > > an attempt to reland an earlier CL seems more suited to part of the CL
> > > description (ie. "The prior CL <foo> was reverted because of <bar>, this
> > > resolves that defect by <baz>"). Otherwise lgtm.
> > 
> > I've adopted your suggestion but in some places where I am reading CL
> > descriptions, that seems like burying the lede. For example, on the bug
> tracker
> > it's going to be difficult to tell the difference between a land and re-land
> if
> > I have to dig deep into the description to figure out what the difference
is.
> > When you revert, "revert" is the first word in the description, which I
think
> is
> > best.
> 
> You haven't changed the issue title here afaict, but maybe that's different
than
> the CL's title? (perhaps the git change title actually comes from the first
line
> of the CL description above? I never actually make those different.) Yeah,
feel
> free to pre-pend "Reland:" (or similar) to the front of CL titles when
> re-landing something that was reverted, I usually do. So I'd actually use a
> title like "Reland: views: Add themed Background support."

(It's also nice when the title isn't cutoff by the character limit)

[Evan Stade, 2017-04-27 20:44:04]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2017-04-27 20:44:04]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2838273002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-04-27 20:44:40]

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-04-27 21:05:40]

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

[commit-bot: I haz the power, 2017-04-27 21:05:42]

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

[Evan Stade, 2017-04-28 21:25:22]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-04-28 21:26:16]

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-04-28 22:28:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493414722708910,
"parent_rev": "a88d3cb58a45213da672a9614c04c949c2e0f215", "commit_rev":
"56fc9d639f67506841a0cb90a421a869e5107eea"}

[commit-bot: I haz the power, 2017-04-28 22:28:18]

Description was changed from

==========
Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

This is a reland of 097f9cde453ea57eb4aa037f44add782391c5eb9 with fix
for asan failure.

BUG=711183, 693282
TBR=stevenjb@chromium.org,tdanderson@chromium.org
==========

to

==========
Introduce a type of View background that stays in sync with its host
View's native theme.

This fixes a few bugs where we weren't using the right native theme,
whether by not updating after the NativeTheme changes or by trying to
access the NativeTheme before the View is added to a hierarchy (which
yields the default NativeTheme --- for most platforms, this didn't
effectively create a bug as there's only one NativeTheme).

Best example is that now the sad tab respects the GTK native theme.

Get rid of Ash's tray_constants::kBackgroundColor in favor of using the
bubble background color from the NativeTheme.

This is a reland of 097f9cde453ea57eb4aa037f44add782391c5eb9 with fix
for asan failure.

BUG=711183, 693282
TBR=stevenjb@chromium.org,tdanderson@chromium.org

Review-Url: https://codereview.chromium.org/2838273002
Cr-Commit-Position: refs/heads/master@{#468157}
Committed:
https://chromium.googlesource.com/chromium/src/+/56fc9d639f67506841a0cb90a421...
==========

[commit-bot: I haz the power, 2017-04-28 22:28:19]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/56fc9d639f67506841a0cb90a421...