Fix tab dragging in unity2d

Fix tab dragging in unity2d

Unity 2d creates non-rectangled or 'shaped' X11 windows. These windows have a
big bounding box according to XGetGeometry but should actually only interact
with part of the screen, which confuses the tab strip dragging code. This
checks if a point is actually inside the ShapeInput list when this extension
is supported by the X display

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

[jamesr, 2012-09-24 23:27:49]

I've manually tested this on unity 2d but not anywhere else yet.

[Elliot Glaysher, 2012-09-24 23:35:05]

https://codereview.chromium.org/10990010/diff/1/ui/base/x/x11_util.cc
File ui/base/x/x11_util.cc (right):

https://codereview.chromium.org/10990010/diff/1/ui/base/x/x11_util.cc#newcode645
ui/base/x/x11_util.cc:645: }
You'll need an XFree(input_rects); here.

[sadrul, 2012-09-24 23:39:24]

https://codereview.chromium.org/10990010/diff/1/ui/base/x/x11_util.cc
File ui/base/x/x11_util.cc (right):

https://codereview.chromium.org/10990010/diff/1/ui/base/x/x11_util.cc#newcode632
ui/base/x/x11_util.cc:632: if (XShapeQueryExtension(ui::GetXDisplay(), &dummy,
&dummy)) {
It would make sense to cache the return value of XShapeQueryExtension

[jamesr, 2012-09-24 23:58:23]

PTAL

[jamesr, 2012-09-25 00:05:22]

FYI this is similar to this code:
http://qt.gitorious.org/qt/qt/commit/33bb996c83e541c26df632b3e8883a1190cc97f0...
which was linked to in
http://code.google.com/p/chromium/issues/detail?id=122789.

[jamesr, 2012-09-25 00:44:39]

Manually checked that this doesn't break tabstrip dragging in unity3d (which
AFAIK doesn't use shapes).  Is there anything else I should check?

I'm guessing chrome/browser/ui/aura/tabs/dock_info_aurax11.cc needs the exact
same patch, but I don't know how to test that.

[Elliot Glaysher, 2012-09-25 16:33:45]

lgtm

On 2012/09/25 00:44:39, jamesr wrote:
> I'm guessing chrome/browser/ui/aura/tabs/dock_info_aurax11.cc needs the exact
> same patch, but I don't know how to test that.

It does. I only realized that after I went home yesterday. Feel free to just
apply the patch to the aurax11 version but not test it.

[sadrul, 2012-09-25 16:37:20]

LGTM with a comment:

http://codereview.chromium.org/10990010/diff/8001/ui/base/x/x11_util.cc
File ui/base/x/x11_util.cc (right):

http://codereview.chromium.org/10990010/diff/8001/ui/base/x/x11_util.cc#newco...
ui/base/x/x11_util.cc:329: static bool is_shape_availability_cached = false;
I think you can just do:

static bool is_shape_available = XShapeQuery...;
return is_shape_available;

That way you can avoid having to use an extra variable (_cached)

[jamesr, 2012-09-25 20:09:46]

On 2012/09/25 16:37:20, sadrul wrote:
> LGTM with a comment:
> 
> http://codereview.chromium.org/10990010/diff/8001/ui/base/x/x11_util.cc
> File ui/base/x/x11_util.cc (right):
> 
>
http://codereview.chromium.org/10990010/diff/8001/ui/base/x/x11_util.cc#newco...
> ui/base/x/x11_util.cc:329: static bool is_shape_availability_cached = false;
> I think you can just do:
> 
> static bool is_shape_available = XShapeQuery...;
> return is_shape_available;
> 
> That way you can avoid having to use an extra variable (_cached)

Want me to do the same for IsRandRAvailable() above?

[sadrul, 2012-09-25 20:12:39]

On 2012/09/25 20:09:46, jamesr wrote:
> On 2012/09/25 16:37:20, sadrul wrote:
> > LGTM with a comment:
> > 
> > http://codereview.chromium.org/10990010/diff/8001/ui/base/x/x11_util.cc
> > File ui/base/x/x11_util.cc (right):
> > 
> >
>
http://codereview.chromium.org/10990010/diff/8001/ui/base/x/x11_util.cc#newco...
> > ui/base/x/x11_util.cc:329: static bool is_shape_availability_cached = false;
> > I think you can just do:
> > 
> > static bool is_shape_available = XShapeQuery...;
> > return is_shape_available;
> > 
> > That way you can avoid having to use an extra variable (_cached)
> 
> Want me to do the same for IsRandRAvailable() above?

That'd be great! Thanks!

[commit-bot: I haz the power, 2012-09-27 08:03:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/10990010/9002

[commit-bot: I haz the power, 2012-09-27 08:03:41]

Presubmit check for 10990010-9002 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Messages **
You might be calling functions intended only for testing from
production code.  It is OK to ignore this warning if you know what
you are doing, as the heuristics used to detect the situation are
not perfect.  The commit queue will not block on this warning.
Email joi@chromium.org if you have questions.
  ui/base/x/x11_util.cc:1440
    void InitXKeyEventForTesting(EventType type,

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui

Presubmit checks took 1.2s to calculate.

[jamesr, 2012-09-27 08:06:59]

Scott, could you OWNERS review this change to chrome/browser/ui/..?  I'm
patching chrome/browser/ui/aura/ with the same fix as in chrome/browser/ui/gtk/
for tabstrip dragging.  erg@ already reviewed the .../gtk/ fix but doesn't have
OWNERS for .../aura/ it appears

[sky, 2012-09-27 15:39:26]

LGTM

[commit-bot: I haz the power, 2012-09-27 18:20:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/10990010/9002

[commit-bot: I haz the power, 2012-09-27 20:53:53]

Change committed as 159111