Mac: Faster scrolling on 10.6.

Mac: Faster scrolling on 10.6.

BUG=45553
TEST=Scrolling up and down still works on 10.5 and 10.6. It uses less %cpu on 10.6.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49442

[Nico, 2010-06-10 18:05:14]

I didn't actually test this on 10.6, but Rohit tested my demo app on the bug,
and Scott tested an earlier version of the patch.

[Mark Mentovai, 2010-06-10 18:48:38]

http://codereview.chromium.org/2771010/diff/2001/3001
File chrome/browser/renderer_host/backing_store_mac.mm (right):

http://codereview.chromium.org/2771010/diff/2001/3001#newcode152
chrome/browser/renderer_host/backing_store_mac.mm:152: bool needsLayerWorkaround
= os_major == 10 && os_minor == 5;
Don’t do this each time through. Check once and maintain a static variable.

C++ style_names in .mm files outside of @interface and @implementation.

[Nico, 2010-06-10 19:57:00]

http://codereview.chromium.org/2771010/diff/2001/3001
File chrome/browser/renderer_host/backing_store_mac.mm (right):

http://codereview.chromium.org/2771010/diff/2001/3001#newcode152
chrome/browser/renderer_host/backing_store_mac.mm:152: bool needsLayerWorkaround
= os_major == 10 && os_minor == 5;
On 2010/06/10 18:48:38, Mark Mentovai wrote:
> Don’t do this each time through. Check once and maintain a static variable.
> 
> C++ style_names in .mm files outside of @interface and @implementation.

Done.

[Mark Mentovai, 2010-06-10 20:05:51]

LGTM

http://codereview.chromium.org/2771010/diff/9001/10001
File chrome/browser/renderer_host/backing_store_mac.mm (right):

http://codereview.chromium.org/2771010/diff/9001/10001#newcode29
chrome/browser/renderer_host/backing_store_mac.mm:29: // Returns if this version
of OS X has broken CGLayers, see
“Returns if” implies that this function doesn’t return if CGLayers aren’t
broken. You should say “Returns whether”.

http://codereview.chromium.org/2771010/diff/9001/10001#newcode35
chrome/browser/renderer_host/backing_store_mac.mm:35: return os_major == 10 &&
os_minor == 5;
For paranoia’s sake:

  return os_major < 10 || (os_major == 10 && os_minor <= 5);

[Nico, 2010-06-10 20:10:30]

Thanks!

http://codereview.chromium.org/2771010/diff/9001/10001
File chrome/browser/renderer_host/backing_store_mac.mm (right):

http://codereview.chromium.org/2771010/diff/9001/10001#newcode29
chrome/browser/renderer_host/backing_store_mac.mm:29: // Returns if this version
of OS X has broken CGLayers, see
On 2010/06/10 20:05:51, Mark Mentovai wrote:
> “Returns if” implies that this function doesn’t return if CGLayers aren’t
> broken. You should say “Returns whether”.

Done.

http://codereview.chromium.org/2771010/diff/9001/10001#newcode35
chrome/browser/renderer_host/backing_store_mac.mm:35: return os_major == 10 &&
os_minor == 5;
On 2010/06/10 20:05:51, Mark Mentovai wrote:
> For paranoia’s sake:
> 
>   return os_major < 10 || (os_major == 10 && os_minor <= 5);

Uh, no :-)

[rohitrao (ping after 24h), 2010-06-10 20:55:08]

http://codereview.chromium.org/2771010/diff/9001/10001
File chrome/browser/renderer_host/backing_store_mac.mm (right):

http://codereview.chromium.org/2771010/diff/9001/10001#newcode35
chrome/browser/renderer_host/backing_store_mac.mm:35: return os_major == 10 &&
os_minor == 5;
On 2010/06/10 20:10:30, Nico wrote:
> On 2010/06/10 20:05:51, Mark Mentovai wrote:
> > For paranoia’s sake:
> > 
> >   return os_major < 10 || (os_major == 10 && os_minor <= 5);
> 
> Uh, no :-)

Why not?  The specificity of checking for 10.5 scares me a bit.  Actually, I
would rather we (semantically) defaulted to the full copy and only implemented
the fast path for >= 10.6.

[Nico, 2010-06-10 21:08:04]

Are you in the CA session? Where do you sit?

http://codereview.chromium.org/2771010/diff/9001/10001
File chrome/browser/renderer_host/backing_store_mac.mm (right):

http://codereview.chromium.org/2771010/diff/9001/10001#newcode35
chrome/browser/renderer_host/backing_store_mac.mm:35: return os_major == 10 &&
os_minor == 5;
On 2010/06/10 20:55:08, rohitrao wrote:
> On 2010/06/10 20:10:30, Nico wrote:
> > On 2010/06/10 20:05:51, Mark Mentovai wrote:
> > > For paranoia’s sake:
> > > 
> > >   return os_major < 10 || (os_major == 10 && os_minor <= 5);
> > 
> > Uh, no :-)
> 
> Why not?  The specificity of checking for 10.5 scares me a bit.  Actually, I
> would rather we (semantically) defaulted to the full copy and only implemented
> the fast path for >= 10.6.

'cause Chrome won't run on 10.4 (or OS 9 for that matter). Hence, this is
functionally equivalent, but without the cargoculting.

[Mark Mentovai, 2010-06-10 22:17:29]

thakis@chromium.org wrote:
> 'cause Chrome won't run on 10.4 (or OS 9 for that matter). Hence, this
> is functionally equivalent, but without the cargoculting.

“Being precise” isn’t really equivalent to my “cargo-culting” in my
mind, but whatever.

Mark