Removed a few manual roundrect path creations

There are several places where we create round rect paths and then draw
the path to the canvas instead of using the canvas's built in round rect
function. Changing these to the canvas round rect could allow skia to
optimize their drawing.

This change switches the status bubble to the canvas round rect.

Committed: https://crrev.com/60dea7f0be1c234f3978679cf438d43b4e9ea04c
Cr-Commit-Position: refs/heads/master@{#295095}

[hendrikw, 2014-09-15 17:46:34]

hendrikw@chromium.org changed reviewers:
+ danakj@chromium.org

[hendrikw, 2014-09-15 17:46:35]

PTAL, Thanks!!

[hendrikw, 2014-09-15 17:48:34]

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

[hendrikw, 2014-09-15 17:48:35]

Hi, PTAL, thanks!

[danakj, 2014-09-15 17:49:52]

https://codereview.chromium.org/568343002/diff/1/ui/gfx/canvas.h
File ui/gfx/canvas.h (right):

https://codereview.chromium.org/568343002/diff/1/ui/gfx/canvas.h#newcode266
ui/gfx/canvas.h:266: int radTL,
style: rad_top_left

[hendrikw, 2014-09-15 17:59:06]

Updated the variable names, thanks!

[sky, 2014-09-15 19:11:18]

https://codereview.chromium.org/568343002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/status_bubble_views.cc (right):

https://codereview.chromium.org/568343002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/status_bubble_views.cc:394: canvas->DrawRoundRect(rect,
radTL, radTR, radBL, radBR, shadow_paint);
This seems to be the only place using this variant of DrawRoundRect. I would
rather keep it here.

[hendrikw, 2014-09-15 19:51:23]

On 2014/09/15 19:11:18, sky wrote:
>
https://codereview.chromium.org/568343002/diff/20001/chrome/browser/ui/views/...
> File chrome/browser/ui/views/status_bubble_views.cc (right):
> 
>
https://codereview.chromium.org/568343002/diff/20001/chrome/browser/ui/views/...
> chrome/browser/ui/views/status_bubble_views.cc:394:
canvas->DrawRoundRect(rect,
> radTL, radTR, radBL, radBR, shadow_paint);
> This seems to be the only place using this variant of DrawRoundRect. I would
> rather keep it here.

canvas->sk_canvas() is const.  I don't have access to its internal function.

[sky, 2014-09-15 21:51:08]

https://codereview.chromium.org/568343002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/status_bubble_views.cc (right):

https://codereview.chromium.org/568343002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/status_bubble_views.cc:394: canvas->DrawRoundRect(rect,
radTL, radTR, radBL, radBR, shadow_paint);
On 2014/09/15 19:11:17, sky wrote:
> This seems to be the only place using this variant of DrawRoundRect. I would
> rather keep it here.

You should be able to use canvas->sk_canvas()->... fine here.

[hendrikw, 2014-09-16 13:42:50]

Oh, I see, a const function that returns a non-const pointer to the internal
implementation of the class.

I assumed no one here would do such a thing.  I'll make another change to make
this function non-const, assuming it isn't already being abused.

I made the change you've requested, but I don't agree with it. It's
inconsistent, leads to messier calling code and might cause others to use the
path method later.

[sky, 2014-09-16 16:37:35]

On 2014/09/16 13:42:50, Hendrik wrote:
> Oh, I see, a const function that returns a non-const pointer to the internal
> implementation of the class.
> 
> I assumed no one here would do such a thing.  I'll make another change to make
> this function non-const, assuming it isn't already being abused.
> 
> I made the change you've requested, but I don't agree with it. It's
> inconsistent, leads to messier calling code and might cause others to use the
> path method later.

gfx::Canvas doesn't offer cover functions for all SkCanvas functions. That's why
the SkCanvas is exposed. The disadvantage of going the route you're suggesting
is we would need to expose functions for every possible SkCanvas function
someone is using. That doesn't scale too well. I like what you have here (and
agree the const returning non-const function is bad).

LGTM

[hendrikw, 2014-09-16 16:38:49]

The CQ bit was checked by hendrikw@chromium.org

[commit-bot: I haz the power, 2014-09-16 16:39:39]

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

[commit-bot: I haz the power, 2014-09-16 17:38:21]

Committed patchset #3 (id:40001) as 782ad076293a24c86d7e9386c387fdbb89c845a3

[commit-bot: I haz the power, 2014-09-16 17:39:47]

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