Remove Gamepad runtime flag (status=stable) for blink

Remove Gamepad runtime flag (status=stable) for blink

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

[heeyoun.lee, 2014-09-19 08:56:26]

heeyoun.lee@samsung.com changed reviewers:
+ eseidel@chromium.org

[heeyoun.lee, 2014-09-19 08:56:27]

This is for blink. PTAL

[heeyoun.lee, 2014-09-19 08:57:57]

On 2014/09/19 08:56:27, heeyoun.lee wrote:
> This is for blink. PTAL

chromium : https://codereview.chromium.org/580403003/

[eseidel, 2014-10-07 16:06:56]

The CQ bit was checked by eseidel@chromium.org

[eseidel, 2014-10-07 16:06:57]

lgtm

https://codereview.chromium.org/582363002/diff/1/Source/web/FrameLoaderClient...
File Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/582363002/diff/1/Source/web/FrameLoaderClient...
Source/web/FrameLoaderClientImpl.cpp:121: NavigatorGamepad::from(*document);
I'm not sure why we're instantiating a NavigatorGamepad here...

[eseidel, 2014-10-07 16:06:57]

The CQ bit was checked by eseidel@chromium.org

[eseidel, 2014-10-07 16:06:58]

lgtm

lgtm

[commit-bot: I haz the power, 2014-10-07 16:07:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/582363002/1

[commit-bot: I haz the power, 2014-10-07 17:22:40]

Committed patchset #1 (id:1) as 183354

[kbalazs, 2014-10-07 18:12:48]

On 2014/10/07 17:22:40, I haz the power (commit-bot) wrote:
> Committed patchset #1 (id:1) as 183354

Sorry guys but I have bad news. This will not work as it did not work before:
https://codereview.chromium.org/560653002/. We need the flag for Jelly Bean.
Heeyoun, could you please add a comment into RuntimeEnabledFeatures.in instead
so that future adventurers will not run into this ;) I am going to revert this.

[kbalazs, 2014-10-07 18:16:24]

On 2014/10/07 18:12:48, kbalazs wrote:
> On 2014/10/07 17:22:40, I haz the power (commit-bot) wrote:
> > Committed patchset #1 (id:1) as 183354
> 
> Sorry guys but I have bad news. This will not work as it did not work before:
> https://codereview.chromium.org/560653002/. We need the flag for Jelly Bean.
> Heeyoun, could you please add a comment into RuntimeEnabledFeatures.in instead
> so that future adventurers will not run into this ;) I am going to revert
this.

Hm, or maybe I'm wrong? Previously it failed on the bots but now it passed. We
don't support 4.3 anymore? Better if I don't take further action without
understanding the situation.

[kbalazs, 2014-10-07 18:32:18]

On 2014/10/07 18:16:24, kbalazs wrote:
> On 2014/10/07 18:12:48, kbalazs wrote:
> > On 2014/10/07 17:22:40, I haz the power (commit-bot) wrote:
> > > Committed patchset #1 (id:1) as 183354
> > 
> > Sorry guys but I have bad news. This will not work as it did not work
before:
> > https://codereview.chromium.org/560653002/. We need the flag for Jelly Bean.
> > Heeyoun, could you please add a comment into RuntimeEnabledFeatures.in
instead
> > so that future adventurers will not run into this ;) I am going to revert
> this.
> 
> Hm, or maybe I'm wrong? Previously it failed on the bots but now it passed. We
> don't support 4.3 anymore? Better if I don't take further action without
> understanding the situation.

Ok, I found the chromium part: https://codereview.chromium.org/580403003/

The problem is that gamepad uses 4.4 api's so now we will actually crash (java
exception) on Jelly Bean. Please light me up if I'm wrong.

[kbalazs, 2014-10-07 20:35:18]

On 2014/10/07 18:32:18, kbalazs wrote:
> On 2014/10/07 18:16:24, kbalazs wrote:
> > On 2014/10/07 18:12:48, kbalazs wrote:
> > > On 2014/10/07 17:22:40, I haz the power (commit-bot) wrote:
> > > > Committed patchset #1 (id:1) as 183354
> > > 
> > > Sorry guys but I have bad news. This will not work as it did not work
> before:
> > > https://codereview.chromium.org/560653002/. We need the flag for Jelly
Bean.
> > > Heeyoun, could you please add a comment into RuntimeEnabledFeatures.in
> instead
> > > so that future adventurers will not run into this ;) I am going to revert
> > this.
> > 
> > Hm, or maybe I'm wrong? Previously it failed on the bots but now it passed.
We
> > don't support 4.3 anymore? Better if I don't take further action without
> > understanding the situation.
> 
> Ok, I found the chromium part: https://codereview.chromium.org/580403003/
> 
> The problem is that gamepad uses 4.4 api's so now we will actually crash (java
> exception) on Jelly Bean. Please light me up if I'm wrong.

Revising my standpoint (and not). It won't crash, just doesn't work. But than we
should not expose the js api if the feature doesn't work.

[kbalazs, 2014-10-07 20:47:59]

On 2014/10/07 20:35:18, kbalazs wrote:
> On 2014/10/07 18:32:18, kbalazs wrote:
> > On 2014/10/07 18:16:24, kbalazs wrote:
> > > On 2014/10/07 18:12:48, kbalazs wrote:
> > > > On 2014/10/07 17:22:40, I haz the power (commit-bot) wrote:
> > > > > Committed patchset #1 (id:1) as 183354
> > > > 
> > > > Sorry guys but I have bad news. This will not work as it did not work
> > before:
> > > > https://codereview.chromium.org/560653002/. We need the flag for Jelly
> Bean.
> > > > Heeyoun, could you please add a comment into RuntimeEnabledFeatures.in
> > instead
> > > > so that future adventurers will not run into this ;) I am going to
revert
> > > this.
> > > 
> > > Hm, or maybe I'm wrong? Previously it failed on the bots but now it
passed.
> We
> > > don't support 4.3 anymore? Better if I don't take further action without
> > > understanding the situation.
> > 
> > Ok, I found the chromium part: https://codereview.chromium.org/580403003/
> > 
> > The problem is that gamepad uses 4.4 api's so now we will actually crash
(java
> > exception) on Jelly Bean. Please light me up if I'm wrong.
> 
> Revising my standpoint (and not). It won't crash, just doesn't work. But than
we
> should not expose the js api if the feature doesn't work.

Now I change my mind...

[16:42] <jamesr__> won't it just return an empty list of gamepads?
[16:43] <kbalazs> jamesr__: yes, I think it will do that. hm, maybe I'm too
paranoid.
[16:44] <jamesr__> sounds like working to me
[16:46] <kbalazs> jamesr__: ok, you buyed me. I was indeed too paranoic.

Sorry for the paranoia.