[Cocoa] Clean-up: Update comments regarding drawRect override in TabContentsContainerView.

[Cocoa] Clean-up: Update comments regarding drawRect override in TabContentsContainerView.

Once upon a time, in an era of great suffering, Chromium Mac was not
using CoreAnimation, with it being necessary to override -drawRect: to
paint the tab contents area background fill.  That time has, unfortunately,
not yet passed.

This change simply updates a few comments to wrap up the resolution
of bug 354598, which ccameron@ has mainly fixed in an earlier
change; and also defaults the bgColor to white when the themed color
is not available.

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

[miu, 2014-06-06 01:09:10]

PTAL.

https://codereview.chromium.org/319703006/diff/1/chrome/browser/ui/cocoa/tab_...
File chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.mm (right):

https://codereview.chromium.org/319703006/diff/1/chrome/browser/ui/cocoa/tab_...
chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.mm:121: bgColor =
[NSColor whiteColor];
The decision (in https://codereview.chromium.org/244043002) was to make the
default whiteColor, so I made this the same as in line 111 above.

https://codereview.chromium.org/319703006/diff/1/chrome/browser/ui/cocoa/tab_...
chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.mm:224:
[contentsContainer setNeedsDisplay:YES];
I kept this call to setNeedsDisplay since this will cause setNeedsDisplayInRect
(line 150ish above) to re-compute the background color.  This is useful both for
making sure the right background color is set each time a tab is unhidden, as
well as when a user theme settings change may necessitate a change in color.

[Avi (use Gerrit), 2014-06-06 02:25:39]

But those days of woe aren't over. We still need the old code path if
kDisableCoreAnimation is used.

Can we short-circuit the overload if we're using CA? I don't want to start
breaking the non-CA path.

[miu, 2014-06-06 06:26:09]

On 2014/06/06 02:25:39, Avi wrote:
> But those days of woe aren't over. We still need the old code path if
> kDisableCoreAnimation is used.
> 
> Can we short-circuit the overload if we're using CA? I don't want to start
> breaking the non-CA path.

Ah...It turns out I did not fully understand how things work.  I put back the
-drawRect: method and added some logging to it in my debug build, and it turns
out the method never gets called when CA is on.  Therefore, no short-circuit is
necessary.  I've put the method back in PS2.

There's not much to this change now.  I'm basically just changing some comments
to wrap-up bug 354598, and making sure the bgColor always defaults to white. 
I've updated the change description to reflect this.

[Avi (use Gerrit), 2014-06-06 14:20:52]

Yeah, this LGTM.

I too am looking forward to removing plenty of non-CA code.

[ccameron, 2014-06-06 16:34:26]

On 2014/06/06 14:20:52, Avi wrote:
> Yeah, this LGTM.
> 
> I too am looking forward to removing plenty of non-CA code.

Oh -- I've started in getting rid of things in the content area. The
--disable-core-animation paths have enough bitrot and bugs that we can go for
this now.

[Avi (use Gerrit), 2014-06-06 16:35:15]

On 2014/06/06 16:34:26, ccameron1 wrote:
> On 2014/06/06 14:20:52, Avi wrote:
> > Yeah, this LGTM.
> > 
> > I too am looking forward to removing plenty of non-CA code.
> 
> Oh -- I've started in getting rid of things in the content area. The
> --disable-core-animation paths have enough bitrot and bugs that we can go for
> this now.

If it doesn't quite work and has bitrotted, why do we keep the flag?

[ccameron, 2014-06-06 16:42:51]

On 2014/06/06 16:35:15, Avi wrote:
> On 2014/06/06 16:34:26, ccameron1 wrote:
> > On 2014/06/06 14:20:52, Avi wrote:
> > > Yeah, this LGTM.
> > > 
> > > I too am looking forward to removing plenty of non-CA code.
> > 
> > Oh -- I've started in getting rid of things in the content area. The
> > --disable-core-animation paths have enough bitrot and bugs that we can go
for
> > this now.
> 
> If it doesn't quite work and has bitrotted, why do we keep the flag?

I'm planning to remove it before M37 branch -- just haven't gotten there yet.

[miu, 2014-06-06 18:27:11]

Okay, so, shall I just land PS2 as-is, and let ccameron@ delete all the non-CA
code in the one big change where the flag is removed?

[ccameron, 2014-06-06 18:32:16]

On 2014/06/06 18:27:11, miu wrote:
> Okay, so, shall I just land PS2 as-is, and let ccameron@ delete all the non-CA
> code in the one big change where the flag is removed?

Sure -- I'll try to get the big removal soon.

[miu, 2014-06-06 19:15:04]

The CQ bit was checked by miu@chromium.org

[commit-bot: I haz the power, 2014-06-06 19:17:08]

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

[commit-bot: I haz the power, 2014-06-07 00:14:11]

Change committed as 275567