Generalize and refactor DeviceSensorEvent* architecture to support multi-event type targets.

Generalize and refactor DeviceSensorEvent* architecture to support
multi-event type targets.

This patch performs a significant refactoring of the
DeviceSensorEvent{Controller,Dispatcher} and related classes.
It provides new Base classes to allow simple implementation of
APIs involving multi-event type non-window targets (e.g. Battery
Status API). Additionally it makes the implementation of existing
event-based APIs (device_orientation, device_light) simpler and
more compact.

In particular the following new classes have been added:
* DeviceEventControllerBase and DeviceEventDispatcherBase
(derived from by the Battery Status API).
* DeviceSingleWindowEventController is more specific and
contains functionality necessary for single-event type window
target APIs (used by Device Motion/Orientation and Device Light
APIs).

BUG=122593, 360068
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175661

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175686

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175792

[Inactive, 2014-06-03 13:25:32]

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
File Source/core/frame/DeviceEventControllerBase.h (right):

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceEventControllerBase.h:2: * Copyright (C) 2014 Google
Inc. All rights reserved.
Please use new copyright headers:
http://dev.chromium.org/blink/coding-style#TOC-License

[Inactive, 2014-06-03 13:49:33]

Here are some comments. Note that I am not a modules OWNER and I think abarth is
away (until he came back very recently). You might want to consider adding
another modules/ OWNER just in case.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
File Source/core/frame/DeviceEventControllerBase.cpp (right):

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceEventControllerBase.cpp:55: m_timer.stop();
Why do we need to stop the timer? It is one-shot.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
File Source/core/frame/DeviceEventControllerBase.h (right):

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceEventControllerBase.h:44: 
nit: Extra line

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
File Source/core/frame/DeviceSingleWindowEventController.h (right):

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:44: 
nit: extra line

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:47: virtual void
didUpdateData() OVERRIDE;
Why does this need to be public?

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:49: // Inherited from
DOMWindowLifecycleObserver.
Why do these need to be public?

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:60: virtual
PassRefPtrWillBeRawPtr<Event> getLastEvent() = 0;
We usually don't use "get" prefix for getters in Blink.
Could be const?

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:61: virtual const
AtomicString& getEventTypeName() = 0;
Ditto.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:62: virtual bool
isNullEvent(Event*) = 0;
Could be const?

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
File Source/modules/battery/BatteryDispatcher.cpp (right):

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryDispatcher.cpp:34: PassRefPtr<BatteryStatus>
BatteryDispatcher::getLatestData()
It seems weird to return a PassRefPtr here since we are not transferring
ownership to the caller. In such cases, the convention in Blink is to return a
raw pointer.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
File Source/modules/battery/BatteryDispatcher.h (right):

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryDispatcher.h:24: PassRefPtr<BatteryStatus>
getLatestData();
We usually don't use "get" prefix for getters in blink (e.g. latestData()). I
know this is not strictly new code so I let you decide if you want to fix it in
this patch.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
File Source/modules/battery/BatteryManager.cpp (right):

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryManager.cpp:31: m_batteryStatus =
BatteryStatus::create();
This could be part of the initializer list to avoid calling RefPtr's default
constructor unnecessarily.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryManager.cpp:60: m_batteryStatus =
BatteryDispatcher::instance().getLatestData();
It is OK to assign a raw ptr to a RefPtr so this should be OK if latestData()
returns a BatteryStatus*.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryManager.cpp:88: bool
BatteryManager::isInvokeInitialUpdate()
This naming seems a bit weird to me but I am not a native English speaker. Does
this mean "isInvokedDuringInitialUpdate?

[timvolodine, 2014-06-03 18:45:53]

thanks for the comments Chris, replies below.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
File Source/core/frame/DeviceEventControllerBase.cpp (right):

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceEventControllerBase.cpp:55: m_timer.stop();
On 2014/06/03 13:49:33, Chris Dumez wrote:
> Why do we need to stop the timer? It is one-shot.

yeah right it's one shot. added an "non-active" assert. Done.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
File Source/core/frame/DeviceEventControllerBase.h (right):

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceEventControllerBase.h:2: * Copyright (C) 2014 Google
Inc. All rights reserved.
On 2014/06/03 13:25:32, Chris Dumez wrote:
> Please use new copyright headers:
> http://dev.chromium.org/blink/coding-style#TOC-License

Done.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceEventControllerBase.h:44: 
On 2014/06/03 13:49:33, Chris Dumez wrote:
> nit: Extra line

Done.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
File Source/core/frame/DeviceSingleWindowEventController.h (right):

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:44: 
On 2014/06/03 13:49:33, Chris Dumez wrote:
> nit: extra line

Done.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:47: virtual void
didUpdateData() OVERRIDE;
On 2014/06/03 13:49:33, Chris Dumez wrote:
> Why does this need to be public?

it's public in the base class and also called by the DeviceEventDispatcherBase.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:49: // Inherited from
DOMWindowLifecycleObserver.
On 2014/06/03 13:49:33, Chris Dumez wrote:
> Why do these need to be public?

they are public in DOMWindowLifecycleObserver.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:60: virtual
PassRefPtrWillBeRawPtr<Event> getLastEvent() = 0;
On 2014/06/03 13:49:33, Chris Dumez wrote:
> We usually don't use "get" prefix for getters in Blink.
> Could be const?

Done.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:61: virtual const
AtomicString& getEventTypeName() = 0;
On 2014/06/03 13:49:33, Chris Dumez wrote:
> Ditto.

Done.

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:62: virtual bool
isNullEvent(Event*) = 0;
On 2014/06/03 13:49:33, Chris Dumez wrote:
> Could be const?

Done.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
File Source/modules/battery/BatteryDispatcher.cpp (right):

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryDispatcher.cpp:34: PassRefPtr<BatteryStatus>
BatteryDispatcher::getLatestData()
On 2014/06/03 13:49:33, Chris Dumez wrote:
> It seems weird to return a PassRefPtr here since we are not transferring
> ownership to the caller. In such cases, the convention in Blink is to return a
> raw pointer.

The idea is to share the RefPtr with the BatteryManager class, i.e.
BatteryManager can keep a reference to this same data. That way no copying is
necessary and the actual refptr would be deleted only if both BatteryManager and
Dispatcher abandon it. For example SVGPathSegList::getItem() seems to perform a
similar approach.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
File Source/modules/battery/BatteryDispatcher.h (right):

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryDispatcher.h:24: PassRefPtr<BatteryStatus>
getLatestData();
On 2014/06/03 13:49:33, Chris Dumez wrote:
> We usually don't use "get" prefix for getters in blink (e.g. latestData()). I
> know this is not strictly new code so I let you decide if you want to fix it
in
> this patch.

Done.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
File Source/modules/battery/BatteryManager.cpp (right):

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryManager.cpp:31: m_batteryStatus =
BatteryStatus::create();
On 2014/06/03 13:49:33, Chris Dumez wrote:
> This could be part of the initializer list to avoid calling RefPtr's default
> constructor unnecessarily.

Done.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryManager.cpp:60: m_batteryStatus =
BatteryDispatcher::instance().getLatestData();
On 2014/06/03 13:49:33, Chris Dumez wrote:
> It is OK to assign a raw ptr to a RefPtr so this should be OK if latestData()
> returns a BatteryStatus*.

hmm, as I wrote previously the idea is to share the same object between
BatteryManager and BatteryDispatcher. If assigning a raw pointer here would wrap
and increase its ref-count that'll work. I am not sure what the preferred way is
to do this, e.g. SVGPathSegList::getItem seems to return a PassRefPtr in such
case..

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryManager.cpp:88: bool
BatteryManager::isInvokeInitialUpdate()
On 2014/06/03 13:49:33, Chris Dumez wrote:
> This naming seems a bit weird to me but I am not a native English speaker.
Does
> this mean "isInvokedDuringInitialUpdate?

what I meant is: if true this class should perform a one-shot update of the data
upon startUpdating(). Not sure how to name it better at this point.

[Inactive, 2014-06-03 18:53:50]

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
File Source/core/frame/DeviceSingleWindowEventController.h (right):

https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
Source/core/frame/DeviceSingleWindowEventController.h:49: // Inherited from
DOMWindowLifecycleObserver.
On 2014/06/03 18:45:54, timvolodine wrote:
> On 2014/06/03 13:49:33, Chris Dumez wrote:
> > Why do these need to be public?
> 
> they are public in DOMWindowLifecycleObserver.

Yes, but as long as these methods are not called on objects of type
DeviceSingleWindowEventController, there is no reason for us to override these
as public, right?

[timvolodine, 2014-06-03 18:59:53]

On 2014/06/03 18:53:50, Chris Dumez wrote:
>
https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
> File Source/core/frame/DeviceSingleWindowEventController.h (right):
> 
>
https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
> Source/core/frame/DeviceSingleWindowEventController.h:49: // Inherited from
> DOMWindowLifecycleObserver.
> On 2014/06/03 18:45:54, timvolodine wrote:
> > On 2014/06/03 13:49:33, Chris Dumez wrote:
> > > Why do these need to be public?
> > 
> > they are public in DOMWindowLifecycleObserver.
> 
> Yes, but as long as these methods are not called on objects of type
> DeviceSingleWindowEventController, there is no reason for us to override these
> as public, right?

technically yes, but since the inheritance is public I thought it's bad practice
to restrict access since each DOMWindowLifecycleObserver should be substitutable
by a DeviceSingle..Controller.

[Inactive, 2014-06-03 19:04:22]

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
File Source/modules/battery/BatteryDispatcher.cpp (right):

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryDispatcher.cpp:34: PassRefPtr<BatteryStatus>
BatteryDispatcher::getLatestData()
On 2014/06/03 18:45:54, timvolodine wrote:
> On 2014/06/03 13:49:33, Chris Dumez wrote:
> > It seems weird to return a PassRefPtr here since we are not transferring
> > ownership to the caller. In such cases, the convention in Blink is to return
a
> > raw pointer.
> 
> The idea is to share the RefPtr with the BatteryManager class, i.e.
> BatteryManager can keep a reference to this same data. That way no copying is
> necessary and the actual refptr would be deleted only if both BatteryManager
and
> Dispatcher abandon it. For example SVGPathSegList::getItem() seems to perform
a
> similar approach.

The fact that you are returning a raw pointer here does not prevent
BatteryManager from storing the return value in a RefPtr. This is a separate
issue. Our convention, as far as I know (I have been corrected in the past on
this matter), is that we return PassRefPtr<> only if we transfer ownership to
the caller (e.g. a factory function). For other cases, we return a raw pointer.
You are going to find some counter-examples in the code base, but it does not
mean that that code is right :)

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
File Source/modules/battery/BatteryManager.cpp (right):

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryManager.cpp:60: m_batteryStatus =
BatteryDispatcher::instance().getLatestData();
On 2014/06/03 18:45:54, timvolodine wrote:
> On 2014/06/03 13:49:33, Chris Dumez wrote:
> > It is OK to assign a raw ptr to a RefPtr so this should be OK if
latestData()
> > returns a BatteryStatus*.
> 
> hmm, as I wrote previously the idea is to share the same object between
> BatteryManager and BatteryDispatcher. If assigning a raw pointer here would
wrap
> and increase its ref-count that'll work. I am not sure what the preferred way
is
> to do this, e.g. SVGPathSegList::getItem seems to return a PassRefPtr in such
> case..

I believe SVGPathSegList::getItem() is a bad example. I still believe we should
return a raw pointer and assign it to a RefPtr here. It will properly increase
the refcount, this is not a problem.

https://codereview.chromium.org/315573002/diff/60001/Source/modules/battery/B...
Source/modules/battery/BatteryManager.cpp:88: bool
BatteryManager::isInvokeInitialUpdate()
On 2014/06/03 18:45:54, timvolodine wrote:
> On 2014/06/03 13:49:33, Chris Dumez wrote:
> > This naming seems a bit weird to me but I am not a native English speaker.
> Does
> > this mean "isInvokedDuringInitialUpdate?
> 
> what I meant is: if true this class should perform a one-shot update of the
data
> upon startUpdating(). Not sure how to name it better at this point.

What was wrong with the previous hasLastData() naming then? It seems clearer to
me.

[Inactive, 2014-06-03 19:08:37]

On 2014/06/03 18:59:53, timvolodine wrote:
> On 2014/06/03 18:53:50, Chris Dumez wrote:
> >
>
https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
> > File Source/core/frame/DeviceSingleWindowEventController.h (right):
> > 
> >
>
https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
> > Source/core/frame/DeviceSingleWindowEventController.h:49: // Inherited from
> > DOMWindowLifecycleObserver.
> > On 2014/06/03 18:45:54, timvolodine wrote:
> > > On 2014/06/03 13:49:33, Chris Dumez wrote:
> > > > Why do these need to be public?
> > > 
> > > they are public in DOMWindowLifecycleObserver.
> > 
> > Yes, but as long as these methods are not called on objects of type
> > DeviceSingleWindowEventController, there is no reason for us to override
these
> > as public, right?
> 
> technically yes, but since the inheritance is public I thought it's bad
practice
> to restrict access since each DOMWindowLifecycleObserver should be
substitutable
> by a DeviceSingle..Controller.

Ok, No big deal either way. I did not know this was considered bad practice. It
is done a lot in Blink.

[timvolodine, 2014-06-04 12:58:26]

On 2014/06/03 19:08:37, Chris Dumez wrote:
> On 2014/06/03 18:59:53, timvolodine wrote:
> > On 2014/06/03 18:53:50, Chris Dumez wrote:
> > >
> >
>
https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
> > > File Source/core/frame/DeviceSingleWindowEventController.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
> > > Source/core/frame/DeviceSingleWindowEventController.h:49: // Inherited
from
> > > DOMWindowLifecycleObserver.
> > > On 2014/06/03 18:45:54, timvolodine wrote:
> > > > On 2014/06/03 13:49:33, Chris Dumez wrote:
> > > > > Why do these need to be public?
> > > > 
> > > > they are public in DOMWindowLifecycleObserver.
> > > 
> > > Yes, but as long as these methods are not called on objects of type
> > > DeviceSingleWindowEventController, there is no reason for us to override
> these
> > > as public, right?
> > 
> > technically yes, but since the inheritance is public I thought it's bad
> practice
> > to restrict access since each DOMWindowLifecycleObserver should be
> substitutable
> > by a DeviceSingle..Controller.
> 
> Ok, No big deal either way. I did not know this was considered bad practice.
It
> is done a lot in Blink.

Alright, looks like the hasLastData suggestion still stands, so I've renamed
"isInvokeInitialUpdate()" to "hasLastData()", thanks for suggesting this.

Also I've fixed BatteryDispatcher::latestData() to return a raw pointer. Would
be good for somebody to double-check.

Chris, does this patch look ok to you now?

[Inactive, 2014-06-04 13:06:58]

On 2014/06/04 12:58:26, timvolodine wrote:
> On 2014/06/03 19:08:37, Chris Dumez wrote:
> > On 2014/06/03 18:59:53, timvolodine wrote:
> > > On 2014/06/03 18:53:50, Chris Dumez wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
> > > > File Source/core/frame/DeviceSingleWindowEventController.h (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/315573002/diff/60001/Source/core/frame/Device...
> > > > Source/core/frame/DeviceSingleWindowEventController.h:49: // Inherited
> from
> > > > DOMWindowLifecycleObserver.
> > > > On 2014/06/03 18:45:54, timvolodine wrote:
> > > > > On 2014/06/03 13:49:33, Chris Dumez wrote:
> > > > > > Why do these need to be public?
> > > > > 
> > > > > they are public in DOMWindowLifecycleObserver.
> > > > 
> > > > Yes, but as long as these methods are not called on objects of type
> > > > DeviceSingleWindowEventController, there is no reason for us to override
> > these
> > > > as public, right?
> > > 
> > > technically yes, but since the inheritance is public I thought it's bad
> > practice
> > > to restrict access since each DOMWindowLifecycleObserver should be
> > substitutable
> > > by a DeviceSingle..Controller.
> > 
> > Ok, No big deal either way. I did not know this was considered bad practice.
> It
> > is done a lot in Blink.
> 
> Alright, looks like the hasLastData suggestion still stands, so I've renamed
> "isInvokeInitialUpdate()" to "hasLastData()", thanks for suggesting this.
> 
> Also I've fixed BatteryDispatcher::latestData() to return a raw pointer. Would
> be good for somebody to double-check.
> 
> Chris, does this patch look ok to you now?

Yes, lgtm. Thanks Tim, nice refactoring.

I hear abarth@ is back so hopefully he can double check.

[timvolodine, 2014-06-05 11:07:51]

adding jochen@ as owner of modules/

[tkent, 2014-06-05 16:06:17]

lgtm

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
File Source/modules/device_orientation/DeviceMotionController.cpp (right):

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
Source/modules/device_orientation/DeviceMotionController.cpp:2: * Copyright (C)
2013 Google Inc. All rights reserved.
If you rewrote this file, please use the 3-line copyright header. Otherwise, we
can't change it.

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
File Source/modules/device_orientation/DeviceMotionController.h (right):

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
Source/modules/device_orientation/DeviceMotionController.h:2: * Copyright (C)
2013 Google Inc. All rights reserved.
If you rewrote this file, please use the 3-line copyright header. Otherwise, we
can't change it.

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
File Source/modules/device_orientation/DeviceOrientationController.cpp (right):

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
Source/modules/device_orientation/DeviceOrientationController.cpp:2: * Copyright
(C) 2013 Google Inc. All rights reserved.
Ditto.

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
File Source/modules/device_orientation/DeviceOrientationController.h (right):

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
Source/modules/device_orientation/DeviceOrientationController.h:2: * Copyright
(C) 2013 Google Inc. All rights reserved.
Ditto.

[timvolodine, 2014-06-05 17:29:04]

thanks!

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
File Source/modules/device_orientation/DeviceMotionController.cpp (right):

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
Source/modules/device_orientation/DeviceMotionController.cpp:2: * Copyright (C)
2013 Google Inc. All rights reserved.
On 2014/06/05 16:06:18, tkent wrote:
> If you rewrote this file, please use the 3-line copyright header. Otherwise,
we
> can't change it.

Done.

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
File Source/modules/device_orientation/DeviceMotionController.h (right):

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
Source/modules/device_orientation/DeviceMotionController.h:2: * Copyright (C)
2013 Google Inc. All rights reserved.
On 2014/06/05 16:06:18, tkent wrote:
> If you rewrote this file, please use the 3-line copyright header. Otherwise,
we
> can't change it.
> 

Done.

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
File Source/modules/device_orientation/DeviceOrientationController.cpp (right):

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
Source/modules/device_orientation/DeviceOrientationController.cpp:2: * Copyright
(C) 2013 Google Inc. All rights reserved.
On 2014/06/05 16:06:18, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
File Source/modules/device_orientation/DeviceOrientationController.h (right):

https://codereview.chromium.org/315573002/diff/120001/Source/modules/device_o...
Source/modules/device_orientation/DeviceOrientationController.h:2: * Copyright
(C) 2013 Google Inc. All rights reserved.
On 2014/06/05 16:06:18, tkent wrote:
> Ditto.

Done.

[timvolodine, 2014-06-05 19:34:20]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2014-06-05 19:35:33]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/timvolodine@chromium.org/315573002/160001

[commit-bot: I haz the power, 2014-06-05 20:50:50]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/7062)
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10835)

[commit-bot: I haz the power, 2014-06-05 22:19:13]

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

[commit-bot: I haz the power, 2014-06-05 22:19:14]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[tkent, 2014-06-05 23:01:38]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2014-06-05 23:02:16]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/timvolodine@chromium.org/315573002/160001

[commit-bot: I haz the power, 2014-06-06 00:18:08]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[commit-bot: I haz the power, 2014-06-06 02:04:08]

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

[commit-bot: I haz the power, 2014-06-06 02:04:09]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[timvolodine, 2014-06-06 09:48:18]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2014-06-06 09:49:10]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/timvolodine@chromium.org/315573002/160001

[commit-bot: I haz the power, 2014-06-06 10:58:15]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[commit-bot: I haz the power, 2014-06-06 12:15:18]

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

[commit-bot: I haz the power, 2014-06-06 12:15:19]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[timvolodine, 2014-06-06 12:18:58]

On 2014/06/06 12:15:19, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   linux_blink_dbg on tryserver.blink
>
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

linux_blink_dbg seems to be broken, all other bots are green, attempting with
NOTRY=true.

[timvolodine, 2014-06-06 12:19:30]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2014-06-06 12:20:20]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/timvolodine@chromium.org/315573002/160001

[commit-bot: I haz the power, 2014-06-06 12:32:49]

Change committed as 175661

[dcheng, 2014-06-06 16:25:16]

A revert of this CL has been created in
https://codereview.chromium.org/315023006/ by dcheng@chromium.org.

The reason for reverting is: Triggers assert in 
fast/dom/Window/property-access-on-cached-window-after-frame-navigated.html.

[timvolodine, 2014-06-06 18:10:51]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2014-06-06 18:11:08]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/timvolodine@chromium.org/315573002/180001

[commit-bot: I haz the power, 2014-06-06 18:20:11]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/7194)

[commit-bot: I haz the power, 2014-06-06 18:22:35]

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

[commit-bot: I haz the power, 2014-06-06 18:22:36]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/7196)

[timvolodine, 2014-06-06 18:24:16]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2014-06-06 18:25:15]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/timvolodine@chromium.org/315573002/180001

[commit-bot: I haz the power, 2014-06-06 18:28:16]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/7197)

[commit-bot: I haz the power, 2014-06-06 18:31:18]

Change committed as 175686

[dcheng, 2014-06-07 07:59:32]

A revert of this CL has been created in
https://codereview.chromium.org/321653003/ by dcheng@chromium.org.

The reason for reverting is: ASAN bots are broken with this patch..

[timvolodine, 2014-06-09 13:09:41]

The CQ bit was checked by timvolodine@chromium.org

[commit-bot: I haz the power, 2014-06-09 13:10:00]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/timvolodine@chromium.org/315573002/200001

[commit-bot: I haz the power, 2014-06-09 13:18:25]

Change committed as 175792