Reenable triggering of screen rotations by sensors.

Reenable triggering of screen rotations by sensors.

Depends on http://codereview.chromium.org/8395046

BUG=None
TEST=Visiting about:rotate?right, about:rotate?left, etc. should cause the appropriate screen rotations.


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

[sadrul, 2011-10-28 16:24:18]

I reviewed the changes in chrome/ and views/. Left some nits.

Is the move of screen_rotation from aura/ to gfx/ necessary for this CL? If not,
I think that should be a separate CL.

http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
File chrome/browser/ui/touch/sensors/screen_orientation_listener.cc (right):

http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
chrome/browser/ui/touch/sensors/screen_orientation_listener.cc:8: #include
"ui/aura/desktop.h"
I believe conditional includes go below the unconditional ones.

http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
chrome/browser/ui/touch/sensors/screen_orientation_listener.cc:77: // Ignore top
and bottom orientations.
It looks like TOP and BOTTOM are handled. Does this comment apply? If it does, I
am not sure what it means. Could you clarify?

http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
File chrome/browser/ui/touch/sensors/screen_orientation_listener.h (right):

http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
chrome/browser/ui/touch/sensors/screen_orientation_listener.h:13: // A singleton
object to manage the virtual keyboard.
"... to manage screen rotation"?

[Ian Vollick, 2011-10-28 17:07:27]

On 2011/10/28 16:24:18, sadrul wrote:
> I reviewed the changes in chrome/ and views/. Left some nits.
> 
> Is the move of screen_rotation from aura/ to gfx/ necessary for this CL? If
not,
> I think that should be a separate CL.
> 
I need to be able to use screen_rotation in both aura desktop and views desktop
modes. Is this possible if it lives in ui/aura/?
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
> File chrome/browser/ui/touch/sensors/screen_orientation_listener.cc (right):
> 
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
> chrome/browser/ui/touch/sensors/screen_orientation_listener.cc:8: #include
> "ui/aura/desktop.h"
> I believe conditional includes go below the unconditional ones.
> 
Fixed.
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
> chrome/browser/ui/touch/sensors/screen_orientation_listener.cc:77: // Ignore
top
> and bottom orientations.
> It looks like TOP and BOTTOM are handled. Does this comment apply? If it does,
I
> am not sure what it means. Could you clarify?
> 
That should have read front and back. Fixed.
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
> File chrome/browser/ui/touch/sensors/screen_orientation_listener.h (right):
> 
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
> chrome/browser/ui/touch/sensors/screen_orientation_listener.h:13: // A
singleton
> object to manage the virtual keyboard.
> "... to manage screen rotation"?
*blush* Fixed.

[sadrul, 2011-10-28 17:17:17]

On 2011/10/28 17:07:27, vollick wrote:
> On 2011/10/28 16:24:18, sadrul wrote:
> > I reviewed the changes in chrome/ and views/. Left some nits.
> > 
> > Is the move of screen_rotation from aura/ to gfx/ necessary for this CL? If
> not,
> > I think that should be a separate CL.
> > 
> I need to be able to use screen_rotation in both aura desktop and views
desktop
> modes. Is this possible if it lives in ui/aura/?

I believe so. views/ can depend on aura/, so this should be fine.

[Ian Vollick, 2011-10-28 20:51:51]

Will keep the screen_rotation code in ui/gfx/compositor so that it is available
when USE_AURA == 0.

[sky, 2011-10-28 21:42:34]

On Fri, Oct 28, 2011 at 10:07 AM,  <vollick@chromium.org> wrote:
> On 2011/10/28 16:24:18, sadrul wrote:
>>
>> I reviewed the changes in chrome/ and views/. Left some nits.
>
>> Is the move of screen_rotation from aura/ to gfx/ necessary for this CL?
>> If
>
> not,
>>
>> I think that should be a separate CL.
>
> I need to be able to use screen_rotation in both aura desktop and views
> desktop
> modes. Is this possible if it lives in ui/aura/?

I like it were you moved it better than in ui/aura.

  -Scott

>
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
>>
>> File chrome/browser/ui/touch/sensors/screen_orientation_listener.cc
>> (right):
>
>
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
>>
>> chrome/browser/ui/touch/sensors/screen_orientation_listener.cc:8: #include
>> "ui/aura/desktop.h"
>> I believe conditional includes go below the unconditional ones.
>
> Fixed.
>
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
>>
>> chrome/browser/ui/touch/sensors/screen_orientation_listener.cc:77: //
>> Ignore
>
> top
>>
>> and bottom orientations.
>> It looks like TOP and BOTTOM are handled. Does this comment apply? If it
>> does,
>
> I
>>
>> am not sure what it means. Could you clarify?
>
> That should have read front and back. Fixed.
>
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
>>
>> File chrome/browser/ui/touch/sensors/screen_orientation_listener.h
>> (right):
>
>
>
http://codereview.chromium.org/8402002/diff/7001/chrome/browser/ui/touch/sens...
>>
>> chrome/browser/ui/touch/sensors/screen_orientation_listener.h:13: // A
>
> singleton
>>
>> object to manage the virtual keyboard.
>> "... to manage screen rotation"?
>
> *blush* Fixed.
>
>
>
> http://codereview.chromium.org/8402002/
>

[Ian Vollick, 2011-10-31 15:01:35]

> >> Is the move of screen_rotation from aura/ to gfx/ necessary for this CL?
> >> If
> >
> > not,
> >>
> >> I think that should be a separate CL.
> >
> > I need to be able to use screen_rotation in both aura desktop and views
> > desktop
> > modes. Is this possible if it lives in ui/aura/?
> 
> I like it were you moved it better than in ui/aura.
> 
>   -Scott

SG -- I've left it under ui/gfx/compositor/. Is there anything else I should
look at with this CL, or does it LGTY?

Thanks,

Ian

[sky, 2011-11-03 22:34:04]

http://codereview.chromium.org/8402002/diff/14001/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

http://codereview.chromium.org/8402002/diff/14001/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:229:
ScreenOrientationListener::GetInstance();
Could this (and the keyboard) be triggered from chrome_browser_parts_touch (you
would have to create it).

[Ian Vollick, 2011-11-04 00:49:23]

On 2011/11/03 22:34:04, sky wrote:
>
http://codereview.chromium.org/8402002/diff/14001/chrome/browser/ui/views/fra...
> File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):
> 
>
http://codereview.chromium.org/8402002/diff/14001/chrome/browser/ui/views/fra...
> chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:229:
> ScreenOrientationListener::GetInstance();
> Could this (and the keyboard) be triggered from chrome_browser_parts_touch
(you
> would have to create it).
Done. The orientation code can certainly be put in _parts_touch class. The
virtual keyboard can be enabled independently of touch_ui, so the _parts_touch
class is not the right home for it. I have left moving the initialization of the
virtual keyboard manager for another CL.

[sky, 2011-11-04 04:15:29]

http://codereview.chromium.org/8402002/diff/18002/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/8402002/diff/18002/chrome/chrome_browser.gypi#...
chrome/chrome_browser.gypi:358: 'browser/chrome_browser_parts_touch.cc',
I think you'll need to exclude these for non-touch platforms. But better than
doing a one-off, you should probably update common.gypi so all _touch files are
excluded if touchui ==0.

[Ian Vollick, 2011-11-04 05:07:58]

On 2011/11/04 04:15:29, sky wrote:
> http://codereview.chromium.org/8402002/diff/18002/chrome/chrome_browser.gypi
> File chrome/chrome_browser.gypi (right):
> 
>
http://codereview.chromium.org/8402002/diff/18002/chrome/chrome_browser.gypi#...
> chrome/chrome_browser.gypi:358: 'browser/chrome_browser_parts_touch.cc',
> I think you'll need to exclude these for non-touch platforms. But better than
> doing a one-off, you should probably update common.gypi so all _touch files
are
> excluded if touchui ==0.

Done.

[sky, 2011-11-04 14:58:16]

LGTM

[commit-bot: I haz the power, 2011-11-07 17:48:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/8402002/25001

[commit-bot: I haz the power, 2011-11-07 19:26:32]

Change committed as 108889