Clip the header of ash windows to the frame height

Clip the header of ash windows to the frame height

Anything painted below the frame are becomes visible if the app enables
the transparent background feature.

For lack of a better way to compute the actual frame height, we clip the
drawing to the height of the close button.

I think ScopedCanvas would come useful here, but it's currently private
to view.cc. If it seems a good idea, I could move it to canvas.h.

BUG=152838
TEST=visually confirmed with patch for bug 125295 applied


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

[sky, 2012-10-17 17:31:09]

Should the clipping be in the app frame code that call into this?

[Bernie, 2012-10-17 23:10:03]

On 2012/10/17 17:31:09, sky wrote:
> Should the clipping be in the app frame code that call into this?

Done. There were three callers:
 ash/wm/custom_frame_view_ash.cc:  frame_painter_->PaintHeader(
 ash/wm/panel_frame_view.cc:  frame_painter_->PaintHeader(
 chrome/browser/ui/views/ash/browser_non_client_frame_view_ash.cc: 
frame_painter_->PaintHeader(

The first two aren't relevant for transparent apps, so I've taken care only of
the last one. If you'd rather fix all three, then I think it makes more sense to
keep the clipping code where it was before.

[Bernie, 2012-10-17 23:10:49]

Please take another look.

[sky, 2012-10-18 16:04:39]

What case is triggering this again?

[Bernie, 2012-10-19 15:51:43]

On 2012/10/18 16:04:39, sky wrote:
> What case is triggering this again?

When an app window is made transparent (see issue 10986092 still under review),
the frame background bleeds down into the client area by about 40 pixels.

Clipping fixes this visual problem and also saves us some extra drawing.

[sky, 2012-10-19 16:53:15]

Is that apps v2 or v1? Shouldn't we be doing this for v2?

  -Scott

On Fri, Oct 19, 2012 at 8:51 AM,  <codewiz@chromium.org> wrote:
> On 2012/10/18 16:04:39, sky wrote:
>>
>> What case is triggering this again?
>
>
> When an app window is made transparent (see issue 10986092 still under
> review),
> the frame background bleeds down into the client area by about 40 pixels.
>
> Clipping fixes this visual problem and also saves us some extra drawing.
>
> https://codereview.chromium.org/11087065/

[Bernie, 2012-10-19 18:15:39]

On 2012/10/19 16:53:15, sky wrote:
> Is that apps v2 or v1? Shouldn't we be doing this for v2?

Yes, it's for apps v2.

[sky, 2012-10-19 21:18:50]

I thought apps v2 wasn't using browser related code?

[Bernie, 2012-12-17 23:35:05]

On 2012/10/19 21:18:50, sky wrote:
> I thought apps v2 wasn't using browser related code?

Indeed, I had not noticed that app windows use CustomFrameViewAsh instead of
BrowserNonClientFrameViewAsh.

In patch set #4, I moved my clipping code to CustomFrameViewAsh. Should we also
clip BrowserNonClientFrameViewAsh to reduce overdraw?

[reveman, 2012-12-18 17:41:14]

rubber stamp lgtm. handing off to sky for review.

[reveman, 2012-12-18 17:42:40]

On 2012/12/17 23:35:05, Bernie wrote:
> On 2012/10/19 21:18:50, sky wrote:
> > I thought apps v2 wasn't using browser related code?
> 
> Indeed, I had not noticed that app windows use CustomFrameViewAsh instead of
> BrowserNonClientFrameViewAsh.
> 
> In patch set #4, I moved my clipping code to CustomFrameViewAsh. Should we
also
> clip BrowserNonClientFrameViewAsh to reduce overdraw?

I don't think we need to worry about that. The compositor occlusion culling code
should prevent overdraw from happening in this case.

[sky, 2012-12-18 20:42:00]

LGTM

[commit-bot: I haz the power, 2012-12-18 22:33:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/codewiz@chromium.org/11087065/21001

[commit-bot: I haz the power, 2012-12-19 01:30:01]

Change committed as 173823