Changes look for scrollbars on windows

Changes look for scrollbars on windows

Additionally respects the size from the OS.

BUG=333499
332937
TEST=none
R=sadrul@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245886

[sky, 2014-01-17 19:34:32]

I had to do a little bit of refactoring to make use of nine painting. I would
have done this in a separate patch, but I'm trying to minimize the number of
patches since this is for 33 (possibly 32).

[sadrul, 2014-01-17 20:11:33]

LGTM

https://codereview.chromium.org/137533012/diff/30001/ui/views/painter.cc
File ui/views/painter.cc (right):

https://codereview.chromium.org/137533012/diff/30001/ui/views/painter.cc#newc...
ui/views/painter.cc:193: : nine_painter_(new gfx::NineImagePainter(image,
insets)) {
It would be really nice if we could use cc::NinePatchLayer for this. But there's
no corresponding ui::Layer for that [yet], and so that is unfortunately
currently not possible.

https://codereview.chromium.org/137533012/diff/30001/ui/views/painter.h
File ui/views/painter.h (right):

https://codereview.chromium.org/137533012/diff/30001/ui/views/painter.h#newco...
ui/views/painter.h:12: #include "ui/base/nine_image_painter_factory.h"
You don't need this here?

... oh, I guess this is so it's not necessary to update all the includes that
currently use IMAGE_GRID?

[sky, 2014-01-17 20:49:34]

https://codereview.chromium.org/137533012/diff/30001/ui/views/painter.h
File ui/views/painter.h (right):

https://codereview.chromium.org/137533012/diff/30001/ui/views/painter.h#newco...
ui/views/painter.h:12: #include "ui/base/nine_image_painter_factory.h"
On 2014/01/17 20:11:33, sadrul wrote:
> You don't need this here?
> 
> ... oh, I guess this is so it's not necessary to update all the includes that
> currently use IMAGE_GRID?

Exactly.

[commit-bot: I haz the power, 2014-01-18 03:11:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/137533012/30001

[commit-bot: I haz the power, 2014-01-18 06:05:18]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-18 17:07:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/137533012/30001

[commit-bot: I haz the power, 2014-01-18 22:35:40]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[sky, 2014-01-19 05:05:59]

+jam or +cpu for change to chrome_main_delegate. I had to make worker processes
on windows load resource bundle as scrollbar drawing (which worker processes may
trigger) needs resource bundle now.

[cpu_(ooo_6.6-7.5), 2014-01-19 18:46:43]

lgtm

[jam, 2014-01-19 18:51:03]

On 2014/01/19 05:05:59, sky wrote:
> +jam or +cpu for change to chrome_main_delegate. I had to make worker
processes
> on windows load resource bundle as scrollbar drawing (which worker processes
may
> trigger) needs resource bundle now.

why would a worker process need this? it only runs JavaScript and doesn't deal
with UI

[sky, 2014-01-19 22:29:39]

On 2014/01/19 18:51:03, jam wrote:
> On 2014/01/19 05:05:59, sky wrote:
> > +jam or +cpu for change to chrome_main_delegate. I had to make worker
> processes
> > on windows load resource bundle as scrollbar drawing (which worker processes
> may
> > trigger) needs resource bundle now.
> 
> why would a worker process need this? it only runs JavaScript and doesn't deal
> with UI

You would think so, eh? See the failures in the 2nd attempt of this patch,
specifically
http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/248553
and test ExtensionDebugger. The failure is the result of the renderer calling
into the NativeTheme engine which needs ResourceBundle. It may not be that the
worker is actually painting, but it definitely queries into NativeTheme which
needs the ResourceBundle to determine sizing information.

Also, based on the other comments in SubprocessNeedsResourceBundle it appears
this isn't that common.

  -Scott

[jam, 2014-01-20 01:03:40]

On 2014/01/19 22:29:39, sky wrote:
> On 2014/01/19 18:51:03, jam wrote:
> > On 2014/01/19 05:05:59, sky wrote:
> > > +jam or +cpu for change to chrome_main_delegate. I had to make worker
> > processes
> > > on windows load resource bundle as scrollbar drawing (which worker
processes
> > may
> > > trigger) needs resource bundle now.
> > 
> > why would a worker process need this? it only runs JavaScript and doesn't
deal
> > with UI
> 
> You would think so, eh? See the failures in the 2nd attempt of this patch,
> specifically
> http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/248553
> and test ExtensionDebugger. The failure is the result of the renderer calling
> into the NativeTheme engine which needs ResourceBundle. It may not be that the
> worker is actually painting, but it definitely queries into NativeTheme which
> needs the ResourceBundle to determine sizing information.
> 
> Also, based on the other comments in SubprocessNeedsResourceBundle it appears
> this isn't that common.
> 
>   -Scott

I see, ok thanks for the explanation. lgtm

[commit-bot: I haz the power, 2014-01-20 06:37:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/137533012/490002

[commit-bot: I haz the power, 2014-01-20 09:18:40]

Change committed as 245886

[jochen (gone - plz use gerrit), 2014-01-20 19:34:16]

A revert of this CL has been created in
https://codereview.chromium.org/141123003/ by jochen@chromium.org.

The reason for reverting is: Broke
webkit_unit_tests:ScrollingCoordinatorChromiumTest.rtlIframe

.