Mac: Optimize TabView drawing.

Mac: Optimize TabView drawing.

This change speeds up -[TabView drawFill:] by about 2x.

Previously each tab had to regenerate a new CGImage for setting the context's
mask every time the tab size changed. This is expensive.
Instead, just fill without the mask and then use NSCompositeDestinationIn to
erase the fill outside the tab's shape. We also save some memory from not
having each tab cache a mask image.

Factor out three part image drawing to a separate class to simplify the code,
and to cache the tab images since we ask for it a lot and ResourceBundle's
cache lookup is not particularly fast.

BUG=453996
Committed: https://crrev.com/15439381be5c6b93dfc80ff0fec4a9d98823dba8
Cr-Commit-Position: refs/heads/master@{#316161}

[Andre, 2015-02-12 04:59:07]

Patchset #1 (id:1) has been deleted

[Andre, 2015-02-12 05:26:25]

andresantoso@chromium.org changed reviewers:
+ rsesek@chromium.org

[Andre, 2015-02-12 05:26:26]

Robert, PTAL and let me know what you think.

[Robert Sesek, 2015-02-12 17:16:47]

I like it. My only major comment is if ThreePartImage should be split out into
its own file so that it can be tested.

https://codereview.chromium.org/918533005/diff/20001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/tabs/tab_view.mm (left):

https://codereview.chromium.org/918533005/diff/20001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/tabs/tab_view.mm:382:
gfx::ScopedNSGraphicsContextSaveGState contextSave;
Why remove this line?

[Andre, 2015-02-12 17:23:01]

Thanks. I will move ThreePartImage out and write some tests. I may break this
into separate CLs.

https://codereview.chromium.org/918533005/diff/20001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/tabs/tab_view.mm (left):

https://codereview.chromium.org/918533005/diff/20001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/tabs/tab_view.mm:382:
gfx::ScopedNSGraphicsContextSaveGState contextSave;
On 2015/02/12 17:16:47, Robert Sesek wrote:
> Why remove this line?

It seems unnecessary.
The documentation for CGContextBeginTransparencyLayer says that the global alpha
is restored
when the transparency layer is ended.

[Andre, 2015-02-12 21:38:56]

Patchset #2 (id:40001) has been deleted

[Andre, 2015-02-12 21:45:19]

Patchset #2 (id:60001) has been deleted

[Andre, 2015-02-12 21:47:10]

Robert, please review patch #2.

[Robert Sesek, 2015-02-12 21:56:38]

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
File ui/base/cocoa/three_part_image.h (right):

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
ui/base/cocoa/three_part_image.h:15: // A wrapper around NSDrawThreePartImage
that caches the images.
This should mention that the ids are for ResourceBundle images.

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
File ui/base/cocoa/three_part_image_unittest.mm (right):

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
ui/base/cocoa/three_part_image_unittest.mm:6: #include
"ui/base/cocoa/three_part_image.h"
This should be the first #include and then separated by a blank line, just like
in the .mm file.

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
ui/base/cocoa/three_part_image_unittest.mm:11: #define EXPECT_EQ_RECT(a, b) \
There's EXPECT_NSRECT_EQ.

[Andre, 2015-02-12 22:10:38]

Robert, PTAL at patchset 3.
I also fixed missing commas in BUILD.gn.

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
File ui/base/cocoa/three_part_image.h (right):

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
ui/base/cocoa/three_part_image.h:15: // A wrapper around NSDrawThreePartImage
that caches the images.
On 2015/02/12 21:56:37, Robert Sesek wrote:
> This should mention that the ids are for ResourceBundle images.

Done.

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
File ui/base/cocoa/three_part_image_unittest.mm (right):

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
ui/base/cocoa/three_part_image_unittest.mm:6: #include
"ui/base/cocoa/three_part_image.h"
On 2015/02/12 21:56:37, Robert Sesek wrote:
> This should be the first #include and then separated by a blank line, just
like
> in the .mm file.

Done.

https://codereview.chromium.org/918533005/diff/80001/ui/base/cocoa/three_part...
ui/base/cocoa/three_part_image_unittest.mm:11: #define EXPECT_EQ_RECT(a, b) \
On 2015/02/12 21:56:37, Robert Sesek wrote:
> There's EXPECT_NSRECT_EQ.

Done.

[Robert Sesek, 2015-02-12 22:13:34]

LGTM

[Andre, 2015-02-12 23:38:47]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2015-02-12 23:39:25]

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

[commit-bot: I haz the power, 2015-02-12 23:48:24]

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

[commit-bot: I haz the power, 2015-02-12 23:48:25]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Andre, 2015-02-12 23:58:33]

andresantoso@chromium.org changed reviewers:
+ thakis@chromium.org

[Andre, 2015-02-12 23:58:34]

Looks like I still need an owner for gyp and gn.
Nico, PTAL.

[Nico, 2015-02-13 04:17:28]

gyp/gn lgtm

The work on the linked bug is awesome!

https://codereview.chromium.org/918533005/diff/100001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/tabs/tab_view.mm (right):

https://codereview.chromium.org/918533005/diff/100001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/tabs/tab_view.mm:51: new
ui::ThreePartImage(IDR_TAB_ALPHA_LEFT, 0, IDR_TAB_ALPHA_RIGHT);
nit: We have CR_DEFINE_STATIC_LOCAL for this (also for the next function)

[Nico, 2015-02-13 04:17:33]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2015-02-13 04:18:26]

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

[commit-bot: I haz the power, 2015-02-13 04:22:07]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2015-02-13 04:22:59]

Patchset 3 (id:??) landed as
https://crrev.com/15439381be5c6b93dfc80ff0fec4a9d98823dba8
Cr-Commit-Position: refs/heads/master@{#316161}

[Andre, 2015-02-13 18:28:16]

On 2015/02/13 04:17:28, Nico wrote:
> gyp/gn lgtm
> 
> The work on the linked bug is awesome!
> 
>
https://codereview.chromium.org/918533005/diff/100001/chrome/browser/ui/cocoa...
> File chrome/browser/ui/cocoa/tabs/tab_view.mm (right):
> 
>
https://codereview.chromium.org/918533005/diff/100001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/tabs/tab_view.mm:51: new
> ui::ThreePartImage(IDR_TAB_ALPHA_LEFT, 0, IDR_TAB_ALPHA_RIGHT);
> nit: We have CR_DEFINE_STATIC_LOCAL for this (also for the next function)

Thanks Nico.
I've uploaded a follow-up CL, https://codereview.chromium.org/922223003/