Use correct ScrollbarTheme to paintScrollCorner

Use correct ScrollbarTheme to paintScrollCorner

This issue is caused by we did not know ScrollbarTheme in
FramePainter::paintScrollCorner then use ScrollbarTheme::theme to
paint.

In this patch, we get ScrollbarTheme from the related scrollbar and
use it to paintScrollCorner.

BUG=676678
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2741223002
Cr-Commit-Position: refs/heads/master@{#456569}
Committed: https://chromium.googlesource.com/chromium/src/+/fe8765486d0c6b4e2e470bf284697fcb9aae5ea4

[chaopeng, 2017-03-10 19:27:33]

Description was changed from

==========
Pass ScrollbarTheme to FramePainter::paintScrollCorner

This issue is caused by we did not know ScrollbarTheme in
FramePainter::paintScrollCorner then use ScrollbarTheme::theme to
paint.

In this patch, we get ScrollbarTheme from the related scrollbar and
pass it to FramePainter::paintScrollCorner.

BUG=676678
==========

to

==========
Pass ScrollbarTheme to FramePainter::paintScrollCorner

This issue is caused by we did not know ScrollbarTheme in
FramePainter::paintScrollCorner then use ScrollbarTheme::theme to
paint.

In this patch, we get ScrollbarTheme from the related scrollbar and
pass it to FramePainter::paintScrollCorner.

BUG=676678
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chaopeng, 2017-03-10 19:27:41]

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

[commit-bot: I haz the power, 2017-03-10 19:28:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chaopeng, 2017-03-10 19:30:23]

chaopeng@chromium.org changed reviewers:
+ bokan@chromium.org

[chaopeng, 2017-03-10 19:30:23]

PTAL. Thank you.

[commit-bot: I haz the power, 2017-03-10 19:53:53]

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

[commit-bot: I haz the power, 2017-03-10 19:53:54]

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

[bokan, 2017-03-13 13:52:09]

https://codereview.chromium.org/2741223002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
(right):

https://codereview.chromium.org/2741223002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:964:
ScrollbarTheme& PaintLayerCompositor::scrollbarThemeOfScrollCorner() const {
FramePainter keeps a reference to frameView itself so you shouldn't need to
determine and pass this in from PaintLayerCompositor. Just determine the theme
from the scrollbars inside FramePainter.

https://codereview.chromium.org/2741223002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/FramePainter.cpp (right):

https://codereview.chromium.org/2741223002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/FramePainter.cpp:220: ScrollbarTheme&
theme,
Rather than passing the theme in, just computed it inside here, rather than
above and in PLC.

[chaopeng, 2017-03-13 20:13:38]

Description was changed from

==========
Pass ScrollbarTheme to FramePainter::paintScrollCorner

This issue is caused by we did not know ScrollbarTheme in
FramePainter::paintScrollCorner then use ScrollbarTheme::theme to
paint.

In this patch, we get ScrollbarTheme from the related scrollbar and
pass it to FramePainter::paintScrollCorner.

BUG=676678
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Use correct ScrollbarTheme to paintScrollCorner

This issue is caused by we did not know ScrollbarTheme in
FramePainter::paintScrollCorner then use ScrollbarTheme::theme to
paint.

In this patch, we get ScrollbarTheme from the related scrollbar and
use it to paintScrollCorner.

BUG=676678
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chaopeng, 2017-03-13 20:13:44]

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

[commit-bot: I haz the power, 2017-03-13 20:14:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chaopeng, 2017-03-13 20:15:34]

Change to get ScrollbarTheme in FramePainter::paintScrollCorner. PTAL. Thank
you.

[commit-bot: I haz the power, 2017-03-13 21:00:44]

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

[commit-bot: I haz the power, 2017-03-13 21:00:45]

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

[bokan, 2017-03-13 21:33:53]

Small style issue but lgtm otherwise

https://codereview.chromium.org/2741223002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/FramePainter.cpp (right):

https://codereview.chromium.org/2741223002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/FramePainter.cpp:239: theme =
&ScrollbarTheme::theme();
style guide says don't handle failing DCHECKs:
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

So if you're confident we never reach this case (I think we should only ever
paint a scroll corner if we have both scrollbars) then remove the theme = ...
line. If there are cases where we hit this then remove the NOTREACHED.

[chaopeng, 2017-03-13 22:10:56]

chaopeng@chromium.org changed reviewers:
+ pdr@google.com

[chaopeng, 2017-03-13 22:15:43]

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

[chaopeng, 2017-03-13 22:16:04]

pdr@ PTAL. Thank you.

[commit-bot: I haz the power, 2017-03-13 22:16:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2017-03-13 22:36:51]

On 2017/03/13 at 22:16:04, chaopeng wrote:
> pdr@ PTAL. Thank you.

This code looks fine to me but I don't understand why other layout tests aren't
failing because of this. Like... why isn't
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/scrollbar...
crashing?

[chaopeng, 2017-03-13 22:52:38]

On 2017/03/13 22:36:51, pdr. wrote:
> On 2017/03/13 at 22:16:04, chaopeng wrote:
> > pdr@ PTAL. Thank you.
> 
> This code looks fine to me but I don't understand why other layout tests
aren't
> failing because of this. Like... why isn't
>
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/scrollbar...
> crashing?

Because this layout test not using overlay scrollbar.  And also I can't make
overlay scrollbar work on layout test.

The crash is at this line.
https://cs.chromium.org/chromium/src/ui/native_theme/native_theme_aura.cc?l=295

[pdr., 2017-03-13 22:59:53]

On 2017/03/13 at 22:52:38, chaopeng wrote:
> On 2017/03/13 22:36:51, pdr. wrote:
> > On 2017/03/13 at 22:16:04, chaopeng wrote:
> > > pdr@ PTAL. Thank you.
> > 
> > This code looks fine to me but I don't understand why other layout tests
aren't
> > failing because of this. Like... why isn't
> >
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/scrollbar...
> > crashing?
> 
> Because this layout test not using overlay scrollbar.  And also I can't make
overlay scrollbar work on layout test.
> 
> The crash is at this line.
>
https://cs.chromium.org/chromium/src/ui/native_theme/native_theme_aura.cc?l=295

Ah I see. OK, LGTM

[commit-bot: I haz the power, 2017-03-14 00:10:33]

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

[commit-bot: I haz the power, 2017-03-14 00:10:34]

Dry run: This issue passed the CQ dry run.

[chaopeng, 2017-03-14 00:47:12]

The CQ bit was checked by chaopeng@chromium.org

[chaopeng, 2017-03-14 00:47:12]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org
Link to the patchset: https://codereview.chromium.org/2741223002/#ps40001
(title: "remove NOTREACHED handler")

[commit-bot: I haz the power, 2017-03-14 00:47: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-03-14 02:04:32]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489452432182990,
"parent_rev": "c0d5ce517c3a43de2eee4e8bd616d2d85ff1a7f9", "commit_rev":
"fe8765486d0c6b4e2e470bf284697fcb9aae5ea4"}

[commit-bot: I haz the power, 2017-03-14 02:05:13]

Description was changed from

==========
Use correct ScrollbarTheme to paintScrollCorner

This issue is caused by we did not know ScrollbarTheme in
FramePainter::paintScrollCorner then use ScrollbarTheme::theme to
paint.

In this patch, we get ScrollbarTheme from the related scrollbar and
use it to paintScrollCorner.

BUG=676678
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Use correct ScrollbarTheme to paintScrollCorner

This issue is caused by we did not know ScrollbarTheme in
FramePainter::paintScrollCorner then use ScrollbarTheme::theme to
paint.

In this patch, we get ScrollbarTheme from the related scrollbar and
use it to paintScrollCorner.

BUG=676678
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2741223002
Cr-Commit-Position: refs/heads/master@{#456569}
Committed:
https://chromium.googlesource.com/chromium/src/+/fe8765486d0c6b4e2e470bf28469...
==========

[commit-bot: I haz the power, 2017-03-14 02:05:14]

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