[SP] Add a drawing recorder before painting caps lock indicator in password fields.

[SP] Add a drawing recorder before painting caps lock indicator in password fields.

Note we only draw a caps lock indicator on Mac OSX.

BUG=501485, 460200
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197427

[wkorman, 2015-06-18 20:03:20]

wkorman@chromium.org changed reviewers:
+ chrishtr@chromium.org

[wkorman, 2015-06-18 20:03:20]

A few notes:
- I don't have a repro yet so can't put together a test.
- It feels strange adding a DrawingRecorder in layout/ code, but there are a
couple of precedents and it is calling paint() here.
- We could move the recorder into ThemePainterMac.mm but having it here seems
more consistent with rest of codebase.

[chrishtr, 2015-06-18 20:09:02]

Also, let's get a test.

https://codereview.chromium.org/1177043016/diff/1/Source/core/layout/LayoutTe...
File Source/core/layout/LayoutTextControlSingleLine.cpp (right):

https://codereview.chromium.org/1177043016/diff/1/Source/core/layout/LayoutTe...
Source/core/layout/LayoutTextControlSingleLine.cpp:89:
LayoutObjectDrawingRecorder recorder(*paintInfo.context, *this, paintInfo.phase,
snappedRect);
visual overflow rect instead?

[wkorman, 2015-06-18 22:26:04]

Added tests.

https://codereview.chromium.org/1177043016/diff/1/Source/core/layout/LayoutTe...
File Source/core/layout/LayoutTextControlSingleLine.cpp (right):

https://codereview.chromium.org/1177043016/diff/1/Source/core/layout/LayoutTe...
Source/core/layout/LayoutTextControlSingleLine.cpp:89:
LayoutObjectDrawingRecorder recorder(*paintInfo.context, *this, paintInfo.phase,
snappedRect);
On 2015/06/18 at 20:09:02, chrishtr wrote:
> visual overflow rect instead?

Discussed in person, sticking with contentsRect as it's been transformed and
should be what's needed for this case. We have risk if painting this ends up
making use of the visual overflow rect down the line, but let's cross that
bridge if we come to it.

[leviw_travelin_and_unemployed, 2015-06-18 22:33:48]

leviw@chromium.org changed reviewers:
+ leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-06-18 22:33:48]

https://codereview.chromium.org/1177043016/diff/20001/Source/core/testing/Int...
File Source/core/testing/Internals.cpp (right):

https://codereview.chromium.org/1177043016/diff/20001/Source/core/testing/Int...
Source/core/testing/Internals.cpp:2442:
PlatformKeyboardEvent::setCurrentCapsLockState(enabled ?
In the past we've had issues with this sort of state outlasting the layout test.
If a test sets this to true, where is it reset to false?

[wkorman, 2015-06-18 22:49:38]

https://codereview.chromium.org/1177043016/diff/20001/Source/core/testing/Int...
File Source/core/testing/Internals.cpp (right):

https://codereview.chromium.org/1177043016/diff/20001/Source/core/testing/Int...
Source/core/testing/Internals.cpp:2442:
PlatformKeyboardEvent::setCurrentCapsLockState(enabled ?
On 2015/06/18 at 22:33:48, leviw wrote:
> In the past we've had issues with this sort of state outlasting the layout
test. If a test sets this to true, where is it reset to false?

Good catch, added to Internals::resetToConsistentState() and tested that
manually running one content shell with two layout tests with --run-layout-test
flag calls the resetToConsistentState() method after each test.

[chrishtr, 2015-06-18 23:21:31]

lgtm

Check strict cull rect before committing.

https://codereview.chromium.org/1177043016/diff/60001/Source/core/testing/Int...
File Source/core/testing/Internals.h (right):

https://codereview.chromium.org/1177043016/diff/60001/Source/core/testing/Int...
Source/core/testing/Internals.h:366: void setCurrentCapsLockState(bool enabled);
setCapsLockState?

[wkorman, 2015-06-18 23:32:30]

The CQ bit was checked by wkorman@chromium.org

[wkorman, 2015-06-18 23:32:30]

https://codereview.chromium.org/1177043016/diff/60001/Source/core/testing/Int...
File Source/core/testing/Internals.h (right):

https://codereview.chromium.org/1177043016/diff/60001/Source/core/testing/Int...
Source/core/testing/Internals.h:366: void setCurrentCapsLockState(bool enabled);
On 2015/06/18 at 23:21:30, chrishtr wrote:
> setCapsLockState?

Done.

[wkorman, 2015-06-18 23:32:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/1177043016/#ps80001
(title: "Rename test method to setCapsLockState.")

[commit-bot: I haz the power, 2015-06-18 23:33:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1177043016/80001

[wkorman, 2015-06-19 02:05:51]

The CQ bit was checked by wkorman@chromium.org

[wkorman, 2015-06-19 02:05:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/1177043016/#ps100001
(title: "Need to rebaseline new tests under virtual/antialiasedtext as well.")

[commit-bot: I haz the power, 2015-06-19 02:06:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1177043016/100001

[commit-bot: I haz the power, 2015-06-19 03:28:46]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197427