Specify clip rects when drawing Mac native widgets

Specify clip rects when drawing Mac native widgets

Add the clip rect as an argument to LocalCurrentGraphicsContext,
and specify the rect for all callers. Add the rect argument for the one
direct caller to SkiaBitLocker (the argument will be made a requirement
once this lands).

De-duplicate a bunch of rectangle-inflating code and move it into
ThemeMac.

For the selected widgets that draw their own focus rings (most don't),
simply specify the maximum inflation of the clip rect as 16 pixels on
each side. This appears to be very generous. The correct API call to
compute this, focusRingMaskBoundsForFrame, was introduced in the
10.7 API, and the widgets that draw their own focus rings wouldn't
need to do so if written against the the 10.7 API.

BUG=247716
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180822

[ccameron, 2014-08-21 05:08:39]

This is to land on top of https://codereview.chromium.org/462423006/, which will
make drawing native widgets with impl-side painting not-terribly-slow.

[eseidel, 2014-08-21 05:13:05]

This looks reasonable.  Unclear if this works correctly with themed controls
which have transforms or scaled applied, but presumably it does since those
should not apply to the passed in rect.

https://codereview.chromium.org/488353003/diff/1/Source/platform/scroll/Scrol...
File Source/platform/scroll/ScrollbarThemeMacOverlayAPI.mm (right):

https://codereview.chromium.org/488353003/diff/1/Source/platform/scroll/Scrol...
Source/platform/scroll/ScrollbarThemeMacOverlayAPI.mm:92:
LocalCurrentGraphicsContext localContext(context, IntRect(0, 0, rect.width(),
rect.height()));
IntRect(IntPoint(), rect.size()) works too.

[ccameron, 2014-08-21 08:50:38]

On 2014/08/21 05:13:05, eseidel wrote:
> This looks reasonable.  Unclear if this works correctly with themed controls
> which have transforms or scaled applied, but presumably it does since those
> should not apply to the passed in rect.

Yes, the clip change is made in the input coordinate system, and is transformed
appropriately by the changes made to the canvas (see, for instance
RenderThemeChromiumMac::paintProgressBar -- we apply the clip in the input
coordinate system, then translate/scale as needed).

That said, there's this really strange thing where, if you zoom (command-plus) a
page with check or radio buttons, they change from native theme to the Aura
theme whenever the scale isn't 100% (see nativeformelements.com for this and
other related fun). Not sure if that is intentional or not, and should probably
be bisected (it goes as far back as beta, IIRC).

https://codereview.chromium.org/488353003/diff/1/Source/platform/scroll/Scrol...
File Source/platform/scroll/ScrollbarThemeMacOverlayAPI.mm (right):

https://codereview.chromium.org/488353003/diff/1/Source/platform/scroll/Scrol...
Source/platform/scroll/ScrollbarThemeMacOverlayAPI.mm:92:
LocalCurrentGraphicsContext localContext(context, IntRect(0, 0, rect.width(),
rect.height()));
On 2014/08/21 05:13:04, eseidel wrote:
> IntRect(IntPoint(), rect.size()) works too.

Done.

[Ken Russell (switch to Gerrit), 2014-08-21 20:43:52]

LGTM as far as I can tell from examining the code. One question.

https://codereview.chromium.org/488353003/diff/20001/Source/platform/mac/Them...
File Source/platform/mac/ThemeMac.mm (right):

https://codereview.chromium.org/488353003/diff/20001/Source/platform/mac/Them...
Source/platform/mac/ThemeMac.mm:221: #if
BUTTON_CELL_DRAW_WITH_FRAME_DRAWS_FOCUS_RING
Is this ever set? codesearch reports one other instance of this symbol, not in
any GYP files.

[ccameron, 2014-08-21 20:58:34]

Thanks!

https://codereview.chromium.org/488353003/diff/20001/Source/platform/mac/Them...
File Source/platform/mac/ThemeMac.mm (right):

https://codereview.chromium.org/488353003/diff/20001/Source/platform/mac/Them...
Source/platform/mac/ThemeMac.mm:221: #if
BUTTON_CELL_DRAW_WITH_FRAME_DRAWS_FOCUS_RING
On 2014/08/21 20:43:52, Ken Russell wrote:
> Is this ever set? codesearch reports one other instance of this symbol, not in
> any GYP files.

CodeSearch has been silently broken for a few days -- the short answer is "it's
defined until we drop 10.6" -- the definition is at

Source/platform/mac/WebCoreNSCellExtras.h:

#define BUTTON_CELL_DRAW_WITH_FRAME_DRAWS_FOCUS_RING
(!defined(MAC_OS_X_VERSION_10_7) || MAC_OS_X_VERSION_MAX_ALLOWED <
MAC_OS_X_VERSION_10_7)

[ccameron, 2014-08-22 19:58:04]

On 2014/08/21 20:58:34, ccameron1 wrote:
> Thanks!
> 
>
https://codereview.chromium.org/488353003/diff/20001/Source/platform/mac/Them...
> File Source/platform/mac/ThemeMac.mm (right):
> 
>
https://codereview.chromium.org/488353003/diff/20001/Source/platform/mac/Them...
> Source/platform/mac/ThemeMac.mm:221: #if
> BUTTON_CELL_DRAW_WITH_FRAME_DRAWS_FOCUS_RING
> On 2014/08/21 20:43:52, Ken Russell wrote:
> > Is this ever set? codesearch reports one other instance of this symbol, not
in
> > any GYP files.
> 
> CodeSearch has been silently broken for a few days -- the short answer is
"it's
> defined until we drop 10.6" -- the definition is at
> 
> Source/platform/mac/WebCoreNSCellExtras.h:
> 
> #define BUTTON_CELL_DRAW_WITH_FRAME_DRAWS_FOCUS_RING
> (!defined(MAC_OS_X_VERSION_10_7) || MAC_OS_X_VERSION_MAX_ALLOWED <
> MAC_OS_X_VERSION_10_7)

Some of the layout test failures are ~1-bit differences due to things that are
clipped being rendered & moved (just rebaseline).

At least one of the layout failures is some other bug on TOT
(fast/frames/iframe-scaling-with-scroll.html)

It also looks like I should pad the region by ~1 pixel to account for
anti-aliasing (fast/forms/placeholder-position.html).

Working on an updated patch.

[ccameron, 2014-08-23 02:54:34]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-08-23 02:55:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/488353003/80001

[commit-bot: I haz the power, 2014-08-23 03:50:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/20891)
  win_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/24054)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/50684)

[commit-bot: I haz the power, 2014-08-23 04:25:40]

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

[commit-bot: I haz the power, 2014-08-23 04:25:41]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/20894)

[ccameron, 2014-08-23 04:55:37]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-08-23 04:56:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/488353003/90010

[commit-bot: I haz the power, 2014-08-24 05:10:30]

Committed patchset #6 (90010) as 180822