Gamepad API: add support for gamepadconnected and gamepaddisconnected events

Gamepad API: add support for gamepadconnected and gamepaddisconnected events

Co-authored with Brandon Jones.

This CL continues the work of updating the Gamepad API to latest spec. NavigatorGamepad implements
WebGamepadListener which is the interface chromium will talk to when dispatching gamepad events.
The implementation follows the dispatcher-controller pattern used by device motion and device orientation.
There is some difference though because the gamepad data is polled instead of pushed by the platform.

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

[kbalazs, 2014-03-14 19:28:16]

I need some help as this adds a call from core to modules which is bad and
should be avoided somehow. Also someone should look carefully at the Oilpan
parts as I'm not very confident with it.

[Inactive, 2014-03-14 19:46:07]

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow.cpp
File Source/core/frame/DOMWindow.cpp (right):

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow....
Source/core/frame/DOMWindow.cpp:108: // FIXME: this is a layering vilation.
Yes, we don't want that :)

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow....
Source/core/frame/DOMWindow.cpp:1530:
NavigatorGamepad::getGamepads(navigator());
You should probably take a look at
FrameLoaderClientImpl::dispatchDidClearWindowObjectInWorld(DOMWrapperWorld*
world) but abarth@ may have a better idea.

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Gamep...
File Source/modules/gamepad/GamepadEvent.h (right):

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Gamep...
Source/modules/gamepad/GamepadEvent.h:9: #include "modules/gamepad/Gamepad.h"
Forward declaration isn't possible?

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
File Source/modules/gamepad/NavigatorGamepad.cpp (right):

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.cpp:140:
frame()->domWindow()->dispatchEvent(event);
We probably need a null check for the frame

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.cpp:152: if (!gamepad ||
gamepad->id().characters16() != webGamepad.id)
Comparison pointers instead of comparing strings? Doesn't look right.

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.cpp:157:
frame()->domWindow()->dispatchEvent(event);
We probably need a null check for the frame

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
File Source/modules/gamepad/NavigatorGamepad.h (right):

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.h:51: virtual void
onGamepadConnected(unsigned index, const blink::WebGamepad&);
OVERRIDE

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.h:52: virtual void
onGamepadDisconnected(unsigned index, const blink::WebGamepad&);
OVERRIDE

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.h:55: NavigatorGamepad(LocalFrame*);
explicit

https://codereview.chromium.org/200783002/diff/1/public/platform/Platform.h
File public/platform/Platform.h (right):

https://codereview.chromium.org/200783002/diff/1/public/platform/Platform.h#n...
public/platform/Platform.h:223: virtual void
setGamepadListener(WebGamepadListener* listener) { }
"listener" can be omitted.

[Inactive, 2014-03-14 19:50:21]

Also, you'll probably need layout testing.

[sof, 2014-03-14 20:52:42]

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow.cpp
File Source/core/frame/DOMWindow.cpp (right):

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow....
Source/core/frame/DOMWindow.cpp:1524: } else if (eventType ==
EventTypeNames::gamepadconnected || eventType ==
EventTypeNames::gamepaddisconnected) {
Just above these special cases, DOMWindow observers are notified about the event
listener being added. I think you can hook in your sampling trigger that way.
Grep for implementations of DOMWindowLifecycleObserver to see how.

[kbalazs, 2014-03-14 23:51:26]

Layering violation solved. For layout testing we will need to extend
WebKitTestRunner which lives in chromium.

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow.cpp
File Source/core/frame/DOMWindow.cpp (right):

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow....
Source/core/frame/DOMWindow.cpp:108: // FIXME: this is a layering vilation.
On 2014/03/14 19:46:08, Chris Dumez wrote:
> Yes, we don't want that :)

Done.

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow....
Source/core/frame/DOMWindow.cpp:1524: } else if (eventType ==
EventTypeNames::gamepadconnected || eventType ==
EventTypeNames::gamepaddisconnected) {
On 2014/03/14 20:52:43, sof wrote:
> Just above these special cases, DOMWindow observers are notified about the
event
> listener being added. I think you can hook in your sampling trigger that way.
> Grep for implementations of DOMWindowLifecycleObserver to see how.

Wonderful, thanks. Together with
FrameLoaderClientImpl::dispatchDidClearWindowObjectInWorld this lead me to a
solution.

https://codereview.chromium.org/200783002/diff/1/Source/core/frame/DOMWindow....
Source/core/frame/DOMWindow.cpp:1530:
NavigatorGamepad::getGamepads(navigator());
On 2014/03/14 19:46:08, Chris Dumez wrote:
> You should probably take a look at
> FrameLoaderClientImpl::dispatchDidClearWindowObjectInWorld(DOMWrapperWorld*
> world) but abarth@ may have a better idea.

Done.

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Gamep...
File Source/modules/gamepad/GamepadEvent.h (right):

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Gamep...
Source/modules/gamepad/GamepadEvent.h:9: #include "modules/gamepad/Gamepad.h"
On 2014/03/14 19:46:08, Chris Dumez wrote:
> Forward declaration isn't possible?

Not really, ~RefPtr has inline call to deref() so it needs the class definition.

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
File Source/modules/gamepad/NavigatorGamepad.cpp (right):

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.cpp:140:
frame()->domWindow()->dispatchEvent(event);
On 2014/03/14 19:46:08, Chris Dumez wrote:
> We probably need a null check for the frame

Yep, null check added for frame() and frame()->domWindow().

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.cpp:152: if (!gamepad ||
gamepad->id().characters16() != webGamepad.id)
On 2014/03/14 19:46:08, Chris Dumez wrote:
> Comparison pointers instead of comparing strings? Doesn't look right.

Oops, I didn't really thought it over. I don't think this condition is needed at
all so I removed.

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.cpp:157:
frame()->domWindow()->dispatchEvent(event);
On 2014/03/14 19:46:08, Chris Dumez wrote:
> We probably need a null check for the frame

Done.

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
File Source/modules/gamepad/NavigatorGamepad.h (right):

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.h:51: virtual void
onGamepadConnected(unsigned index, const blink::WebGamepad&);
On 2014/03/14 19:46:08, Chris Dumez wrote:
> OVERRIDE

Done.

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.h:52: virtual void
onGamepadDisconnected(unsigned index, const blink::WebGamepad&);
On 2014/03/14 19:46:08, Chris Dumez wrote:
> OVERRIDE

Done.

https://codereview.chromium.org/200783002/diff/1/Source/modules/gamepad/Navig...
Source/modules/gamepad/NavigatorGamepad.h:55: NavigatorGamepad(LocalFrame*);
On 2014/03/14 19:46:08, Chris Dumez wrote:
> explicit

Done.

https://codereview.chromium.org/200783002/diff/1/public/platform/Platform.h
File public/platform/Platform.h (right):

https://codereview.chromium.org/200783002/diff/1/public/platform/Platform.h#n...
public/platform/Platform.h:223: virtual void
setGamepadListener(WebGamepadListener* listener) { }
On 2014/03/14 19:46:08, Chris Dumez wrote:
> "listener" can be omitted.

Done.

[abarth-chromium, 2014-03-17 18:23:46]

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
File Source/modules/gamepad/NavigatorGamepad.cpp (right):

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:81:
blink::Platform::current()->setGamepadListener(0);
This line sets a static field in the embedder.  How can this work given that
there can be many instances of NavigatorGamepad, one for each iframe?

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:168:
blink::Platform::current()->setGamepadListener(this);
Again, this sets a static field, which doesn't seem like it can work.

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
File Source/modules/gamepad/NavigatorGamepad.h (right):

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.h:41: class NavigatorGamepad FINAL :
public Supplement<Navigator>, public DOMWindowProperty, public
blink::WebGamepadListener, public DOMWindowLifecycleObserver {
You shouldn't need both DOMWindowProperty and DOMWindowLifecycleObserver. 
DOMWindowProperty already gives you all the lifecycle observations you should
need.

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.h:53: virtual void
onGamepadDisconnected(unsigned index, const blink::WebGamepad&) OVERRIDE;
We use Obj-C style names:

didConnectGamepad
willDisconnectGamepad

(whichever is appropriate)

https://codereview.chromium.org/200783002/diff/40001/public/platform/WebGamep...
File public/platform/WebGamepads.h (right):

https://codereview.chromium.org/200783002/diff/40001/public/platform/WebGamep...
public/platform/WebGamepads.h:42: };
Please put new classes into their own headers.

[kbalazs, 2014-03-17 23:49:27]

Comments incorporated, PTAL.

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
File Source/modules/gamepad/NavigatorGamepad.cpp (right):

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:81:
blink::Platform::current()->setGamepadListener(0);
On 2014/03/17 18:23:46, abarth wrote:
> This line sets a static field in the embedder.  How can this work given that
> there can be many instances of NavigatorGamepad, one for each iframe?

Added a singleton mediator that implements WebGamepadListener and forwards the
virtual calls to each NavigatorGamepad that wants to listen. We really just need
to set a listener for the renderer once so I think this is sufficient.

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:168:
blink::Platform::current()->setGamepadListener(this);
On 2014/03/17 18:23:46, abarth wrote:
> Again, this sets a static field, which doesn't seem like it can work.

Done.

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
File Source/modules/gamepad/NavigatorGamepad.h (right):

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.h:41: class NavigatorGamepad FINAL :
public Supplement<Navigator>, public DOMWindowProperty, public
blink::WebGamepadListener, public DOMWindowLifecycleObserver {
On 2014/03/17 18:23:46, abarth wrote:
> You shouldn't need both DOMWindowProperty and DOMWindowLifecycleObserver. 
> DOMWindowProperty already gives you all the lifecycle observations you should
> need.

I don't need both indeed. I want to start listening from the time when
addWindowListener is used. This is to not ask the browser process to start the
gamepad code before it is needed because it has a cost (creating the polling
thread). So I kept the inheritance from DOMWindowLifecycleObserver and removed
DOMWindowProperty.

https://codereview.chromium.org/200783002/diff/40001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.h:53: virtual void
onGamepadDisconnected(unsigned index, const blink::WebGamepad&) OVERRIDE;
On 2014/03/17 18:23:46, abarth wrote:
> We use Obj-C style names:
> 
> didConnectGamepad
> willDisconnectGamepad
> 
> (whichever is appropriate)

Changed to didConnectGamepad and didDisconnectGamepad.

https://codereview.chromium.org/200783002/diff/40001/public/platform/WebGamep...
File public/platform/WebGamepads.h (right):

https://codereview.chromium.org/200783002/diff/40001/public/platform/WebGamep...
public/platform/WebGamepads.h:42: };
On 2014/03/17 18:23:46, abarth wrote:
> Please put new classes into their own headers.

Done.

[abarth-chromium, 2014-03-19 18:00:32]

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
File Source/modules/gamepad/NavigatorGamepad.cpp (right):

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:42: class GlobalGamepadListener :
public blink::WebGamepadListener {
Can you put this class in its own file?  For consistency with other modules, we
might want to rename GlobalGamepadListener to GamepadDispatcher

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:59:
blink::Platform::current()->setGamepadListener(this);
If you look at how DeviceOrientationDispatcher, you'll see that we usually use a
pattern that lets us unregister for this notifications when there aren't any
more listeners.  Hopefully that will save battery once no one cares about these
events.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:75: (*it)->didConnectGamepad(index,
gamepad);
What stops m_clients from being mutated during this iteration?  We're running
arbitrary script here, which means someone could add or remove a client.  Take a
look at how the DeviceOrientationDispatcher solves this problem.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:130:
globalListener().removeClient(this);
The problem with only calling removeClient in the destructor is that we'll leak
information to the web page about when the destructor is called.  That's
dependent on garbage collection, which means we'll leak information about when
GC occurs to the web site.  Instead, we should unregister for these
notifications at a defined point in the lifecycle.  Take a look at how device
orientation solves this problem.  My guess is that we're subclassing
DOMWindowProperty and stop listening when the DOMWindow disconnects from the
frame.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:177: return;
How could index >= blink::WebGamepads::itemsLengthCap?  That seems like it ought
to be an assert...

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:188: if (window()) {
Do we want to do the work above if we don't have a window?  It seems like once
we lose the connection to the window, we should stop sampling the gamepad.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
File Source/modules/gamepad/NavigatorGamepad.h (right):

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.h:30: #include
"core/frame/DOMWindowProperty.h"
Looks like you don't need this include.

[kbalazs, 2014-03-20 18:57:20]

Reworked to use DeviceSensorEventDispatcher and DeviceSensorEventController.
PTAL.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
File Source/modules/gamepad/NavigatorGamepad.cpp (right):

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:59:
blink::Platform::current()->setGamepadListener(this);
On 2014/03/19 18:00:33, abarth wrote:
> If you look at how DeviceOrientationDispatcher, you'll see that we usually use
a
> pattern that lets us unregister for this notifications when there aren't any
> more listeners.  Hopefully that will save battery once no one cares about
these
> events.

Good point. After looking at orientation and motion I realized that the
requirements here are similar and it makes sense to reuse the same pattern. So
now NavigatorGamepad is a DeviceSensorEventController and GamepadDispatcher is a
DeviceSensorEventDispatcher. There is some difference in how I implemented the
virtual methods though because with gamepad a script can sample gamepads without
first adding the event listener.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:75: (*it)->didConnectGamepad(index,
gamepad);
On 2014/03/19 18:00:33, abarth wrote:
> What stops m_clients from being mutated during this iteration?  We're running
> arbitrary script here, which means someone could add or remove a client.  Take
a
> look at how the DeviceOrientationDispatcher solves this problem.

Solved by using DeviceSensorDispatcher and DeviceSensorEventController.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:130:
globalListener().removeClient(this);
On 2014/03/19 18:00:33, abarth wrote:
> The problem with only calling removeClient in the destructor is that we'll
leak
> information to the web page about when the destructor is called.  That's
> dependent on garbage collection, which means we'll leak information about when
> GC occurs to the web site.  Instead, we should unregister for these
> notifications at a defined point in the lifecycle.  Take a look at how device
> orientation solves this problem.  My guess is that we're subclassing
> DOMWindowProperty and stop listening when the DOMWindow disconnects from the
> frame.

Orientation and motion unregisters when the event listener is removed. But this
doesn't work here because the gamepads can be sampled without event listeners so
I again inherit from DOMWindowProperty as well and unregister in
willDestroyGlobalObjectInFrame and willDetachGlobalObjectInFrame.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:177: return;
On 2014/03/19 18:00:33, abarth wrote:
> How could index >= blink::WebGamepads::itemsLengthCap?  That seems like it
ought
> to be an assert...

Done.

https://codereview.chromium.org/200783002/diff/60001/Source/modules/gamepad/N...
Source/modules/gamepad/NavigatorGamepad.cpp:188: if (window()) {
On 2014/03/19 18:00:33, abarth wrote:
> Do we want to do the work above if we don't have a window?  It seems like once
> we lose the connection to the window, we should stop sampling the gamepad.

Done.

[sof, 2014-03-20 19:54:39]

https://codereview.chromium.org/200783002/diff/80001/Source/modules/gamepad/G...
File Source/modules/gamepad/GamepadEvent.h (right):

https://codereview.chromium.org/200783002/diff/80001/Source/modules/gamepad/G...
Source/modules/gamepad/GamepadEvent.h:46: RefPtrWillBeMember<Gamepad> m_gamepad;
Please use RefPtrWillBePersistent as Events aren't garbage collected yet, so we
need to keep a strong reference to the object across GCs.

[abarth-chromium, 2014-03-20 20:10:09]

LGTM modulo sof's comment

https://codereview.chromium.org/200783002/diff/80001/public/platform/Platform.h
File public/platform/Platform.h (right):

https://codereview.chromium.org/200783002/diff/80001/public/platform/Platform...
public/platform/Platform.h:221: 
Please keep two blank lines between sections.

[kbalazs, 2014-03-20 22:03:09]

The CQ bit was checked by b.kelemen@samsung.com

[commit-bot: I haz the power, 2014-03-20 22:03:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/200783002/100001

[commit-bot: I haz the power, 2014-03-20 22:15:28]

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

[commit-bot: I haz the power, 2014-03-20 22:15:29]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_compile_dbg

[kbalazs, 2014-03-20 22:38:34]

The CQ bit was checked by b.kelemen@samsung.com

[commit-bot: I haz the power, 2014-03-20 22:38:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/200783002/120001

[commit-bot: I haz the power, 2014-03-20 22:39:56]

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

[commit-bot: I haz the power, 2014-03-20 22:39:57]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_rel

[bajones, 2014-03-20 22:42:14]

Just wanted to say thanks to kbalazs@ for carrying this forward. A lot of the
changes requested were actually issues with the original code that I handed off,
and I'm grateful that he's taken the time to beat it into commit-worthy shape
while I've been otherwise occupied.

Great work! :)

[kbalazs, 2014-03-20 23:16:03]

The CQ bit was checked by b.kelemen@samsung.com

[commit-bot: I haz the power, 2014-03-20 23:16:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/200783002/120001

[commit-bot: I haz the power, 2014-03-20 23:16:38]

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

[commit-bot: I haz the power, 2014-03-20 23:16:39]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_rel

[kbalazs, 2014-03-20 23:16:48]

On 2014/03/20 22:42:14, bajones wrote:
> Just wanted to say thanks to kbalazs@ for carrying this forward. A lot of the
> changes requested were actually issues with the original code that I handed
off,
> and I'm grateful that he's taken the time to beat it into commit-worthy shape
> while I've been otherwise occupied.
> 
> Great work! :)

Sure thing. Working on this feature is definitely fun. :)

[kbalazs, 2014-03-20 23:26:32]

The CQ bit was checked by b.kelemen@samsung.com

[commit-bot: I haz the power, 2014-03-20 23:26:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/200783002/140001

[commit-bot: I haz the power, 2014-03-20 23:28:56]

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

[commit-bot: I haz the power, 2014-03-20 23:28:57]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_compile_dbg

[kbalazs, 2014-03-21 02:38:33]

The CQ bit was checked by b.kelemen@samsung.com

[commit-bot: I haz the power, 2014-03-21 02:38:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/b.kelemen@samsung.com/200783002/140001

[commit-bot: I haz the power, 2014-03-21 02:57:51]

Change committed as 169716