Remove gfx::Canvas::DrawStringRectWithHalo.

Remove gfx::Canvas::DrawStringRectWithHalo.

This was not used anywhere in production. We have a faux halo effect in
a couple places (dragged bookmarks, avatar button) which is accomplished
with stacked shadows. This is something I might try to change, but I
don't believe the current halo code is going to help with that.

The only current uses of DrawStringRectWithHalo are in tests and example
code which can be removed without harm.

BUG=624175
Review-Url: https://codereview.chromium.org/2654853002
Cr-Commit-Position: refs/heads/master@{#459830}
Committed: https://chromium.googlesource.com/chromium/src/+/2a2a1f9d36c51c3a4762cd7ddaf9d4a2d142ca24

[Evan Stade, 2017-01-24 17:19:35]

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

[Evan Stade, 2017-01-24 17:19:45]

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

[commit-bot: I haz the power, 2017-01-24 17:20:07]

Dry run: 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-01-24 17:59:41]

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

[commit-bot: I haz the power, 2017-01-24 17:59:42]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[msw, 2017-01-25 18:09:07]

lgtm with nits

https://codereview.chromium.org/2654853002/diff/1/ui/compositor/layer_unittes...
File ui/compositor/layer_unittest.cc (right):

https://codereview.chromium.org/2654853002/diff/1/ui/compositor/layer_unittes...
ui/compositor/layer_unittest.cc:98: enum DrawFunction {
Please remove this enum with only one value

https://codereview.chromium.org/2654853002/diff/1/ui/gfx/render_text.cc
File ui/gfx/render_text.cc (right):

https://codereview.chromium.org/2654853002/diff/1/ui/gfx/render_text.cc#newco...
ui/gfx/render_text.cc:26: #include
"third_party/skia/include/effects/SkMorphologyImageFilter.h"
nit: remove

https://codereview.chromium.org/2654853002/diff/1/ui/gfx/render_text.cc#newco...
ui/gfx/render_text.cc:863: internal::SkiaTextRenderer renderer(canvas);
nit: inline |renderer| and remove curlies

[Evan Stade, 2017-01-26 00:43:11]

oh dang, I guess it is still used on Windows (which sadly doesn't show up on cs
or with a local linux/cros compile). See drag_utils.cc

Do you think we should keep it as-is or make it achieve the halo in the same way
as dragged bookmarks/avatar button?

https://codereview.chromium.org/2654853002/diff/1/ui/compositor/layer_unittes...
File ui/compositor/layer_unittest.cc (right):

https://codereview.chromium.org/2654853002/diff/1/ui/compositor/layer_unittes...
ui/compositor/layer_unittest.cc:98: enum DrawFunction {
On 2017/01/25 18:09:07, msw wrote:
> Please remove this enum with only one value

Done.

https://codereview.chromium.org/2654853002/diff/1/ui/gfx/render_text.cc
File ui/gfx/render_text.cc (right):

https://codereview.chromium.org/2654853002/diff/1/ui/gfx/render_text.cc#newco...
ui/gfx/render_text.cc:26: #include
"third_party/skia/include/effects/SkMorphologyImageFilter.h"
On 2017/01/25 18:09:07, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/2654853002/diff/1/ui/gfx/render_text.cc#newco...
ui/gfx/render_text.cc:863: internal::SkiaTextRenderer renderer(canvas);
On 2017/01/25 18:09:07, msw wrote:
> nit: inline |renderer| and remove curlies

I don't think I can do that --- how does one take the address of a temporary
without breaking the compile?

[msw, 2017-01-26 00:52:10]

On 2017/01/26 00:43:11, Evan Stade wrote:
> oh dang, I guess it is still used on Windows (which sadly doesn't show up on
cs
> or with a local linux/cros compile). See drag_utils.cc
> 
> Do you think we should keep it as-is or make it achieve the halo in the same
way
> as dragged bookmarks/avatar button?

Yeah, the download button drag image should match the bookmarks button drag
image, feel free to change that.

https://codereview.chromium.org/2654853002/diff/1/ui/gfx/render_text.cc
File ui/gfx/render_text.cc (right):

https://codereview.chromium.org/2654853002/diff/1/ui/gfx/render_text.cc#newco...
ui/gfx/render_text.cc:863: internal::SkiaTextRenderer renderer(canvas);
On 2017/01/26 00:43:10, Evan Stade wrote:
> On 2017/01/25 18:09:07, msw wrote:
> > nit: inline |renderer| and remove curlies
> 
> I don't think I can do that --- how does one take the address of a temporary
> without breaking the compile?

Ah, I missed that; disregard my comment.

https://codereview.chromium.org/2654853002/diff/20001/ui/compositor/layer_uni...
File ui/compositor/layer_unittest.cc (right):

https://codereview.chromium.org/2654853002/diff/20001/ui/compositor/layer_uni...
ui/compositor/layer_unittest.cc:96: class DrawFadedStringLayerDelegateDelegate :
public LayerDelegate {
nit: DelegateDelegate?

[Evan Stade, 2017-03-22 22:59:49]

Description was changed from

==========
Remove gfx::Canvas::DrawStringRectWithHalo.

This was not used anywhere in production. We have a faux halo effect in
a couple places (dragged bookmarks, avatar button) which is accomplished
with stacked shadows. This is something I might try to change, but I
don't believe the current halo code is going to help with that.

The only current uses of DrawStringRectWithHalo are in tests and example
code which can be removed without harm.

BUG=624175
==========

to

==========
Remove gfx::Canvas::DrawStringRectWithHalo.

This was not used anywhere in production. We have a faux halo effect in
a couple places (dragged bookmarks, avatar button) which is accomplished
with stacked shadows. This is something I might try to change, but I
don't believe the current halo code is going to help with that.

The only current uses of DrawStringRectWithHalo are in tests and example
code which can be removed without harm.

BUG=624175
==========

[Evan Stade, 2017-03-22 22:59:49]

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

[Evan Stade, 2017-03-22 22:59:55]

estade@chromium.org changed reviewers:
- sky@chromium.org

[Evan Stade, 2017-03-22 22:59:56]

rebased, +sky for layer_unittest.cc

https://codereview.chromium.org/2654853002/diff/20001/ui/compositor/layer_uni...
File ui/compositor/layer_unittest.cc (right):

https://codereview.chromium.org/2654853002/diff/20001/ui/compositor/layer_uni...
ui/compositor/layer_unittest.cc:96: class DrawFadedStringLayerDelegateDelegate :
public LayerDelegate {
On 2017/01/26 00:52:10, msw wrote:
> nit: DelegateDelegate?

Done.

[Evan Stade, 2017-03-22 23:00:07]

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

[Evan Stade, 2017-03-22 23:00:07]

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

[Evan Stade, 2017-03-22 23:00:20]

actually +sky. PTAL layer_unittest.cc

[msw, 2017-03-22 23:20:11]

still lgtm

[sky, 2017-03-22 23:50:53]

LGTM

[Evan Stade, 2017-03-23 12:38:12]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-03-23 12:38:23]

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-03-23 13:07:28]

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

[commit-bot: I haz the power, 2017-03-23 13:07:30]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Evan Stade, 2017-03-27 17:03:34]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2017-03-27 17:03:34]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2654853002/#ps60001
(title: "fix test")

[commit-bot: I haz the power, 2017-03-27 17:04:04]

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-03-27 18:20:13]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490634214050280,
"parent_rev": "1697bb35e2553952e934208ebbca82af75a15c89", "commit_rev":
"2a2a1f9d36c51c3a4762cd7ddaf9d4a2d142ca24"}

[commit-bot: I haz the power, 2017-03-27 18:20:53]

Description was changed from

==========
Remove gfx::Canvas::DrawStringRectWithHalo.

This was not used anywhere in production. We have a faux halo effect in
a couple places (dragged bookmarks, avatar button) which is accomplished
with stacked shadows. This is something I might try to change, but I
don't believe the current halo code is going to help with that.

The only current uses of DrawStringRectWithHalo are in tests and example
code which can be removed without harm.

BUG=624175
==========

to

==========
Remove gfx::Canvas::DrawStringRectWithHalo.

This was not used anywhere in production. We have a faux halo effect in
a couple places (dragged bookmarks, avatar button) which is accomplished
with stacked shadows. This is something I might try to change, but I
don't believe the current halo code is going to help with that.

The only current uses of DrawStringRectWithHalo are in tests and example
code which can be removed without harm.

BUG=624175

Review-Url: https://codereview.chromium.org/2654853002
Cr-Commit-Position: refs/heads/master@{#459830}
Committed:
https://chromium.googlesource.com/chromium/src/+/2a2a1f9d36c51c3a4762cd7ddaf9...
==========

[commit-bot: I haz the power, 2017-03-27 18:20:55]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/2a2a1f9d36c51c3a4762cd7ddaf9...