Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

Intent to Remove thread:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/mW1njtgDPHA

BUG=689681
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2795223002
Cr-Commit-Position: refs/heads/master@{#466930}
Committed: https://chromium.googlesource.com/chromium/src/+/0363d88c658806d011c31a4f48f25779e9bc5f22

[Shanmuga Pandi, 2017-04-04 13:38:16]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 13:38:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 14:53:54]

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

[commit-bot: I haz the power, 2017-04-04 14:53:59]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Shanmuga Pandi, 2017-04-05 05:08:11]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-05 05:08:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Shanmuga Pandi, 2017-04-05 06:18:29]

shanmuga.m@samsung.com changed reviewers:
+ foolip@chromium.org, fs@opera.com, meade@chromium.org

[Shanmuga Pandi, 2017-04-05 06:18:30]

PTAL!!

[commit-bot: I haz the power, 2017-04-05 06:54:30]

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

[commit-bot: I haz the power, 2017-04-05 06:54:31]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fs, 2017-04-05 10:21:01]

https://codereview.chromium.org/2795223002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSRule.idl (left):

https://codereview.chromium.org/2795223002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSRule.idl:54: const unsigned short
WEBKIT_KEYFRAMES_RULE = 7;
I guess you should also remove the internal values?
(CSSRule::Type::kWebkitKeyframe{,s}Rule)

[Shanmuga Pandi, 2017-04-05 10:46:22]

https://codereview.chromium.org/2795223002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSRule.idl (left):

https://codereview.chromium.org/2795223002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSRule.idl:54: const unsigned short
WEBKIT_KEYFRAMES_RULE = 7;
On 2017/04/05 10:21:01, fs wrote:
> I guess you should also remove the internal values?
> (CSSRule::Type::kWebkitKeyframe{,s}Rule)

Done.

[fs, 2017-04-05 10:54:42]

LGTM, but you'll need someone to sign off on the webexposed bits. I'll also
leave to them to determine how much additional red tape this will require
(WebKit still exposes these.)

[Shanmuga Pandi, 2017-04-05 11:08:09]

Description was changed from

==========
Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

BUG=689681
==========

to

==========
Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

BUG=689681
==========

[Shanmuga Pandi, 2017-04-05 11:08:09]

shanmuga.m@samsung.com changed reviewers:
+ timvolodine@chromium.org

[Shanmuga Pandi, 2017-04-05 11:08:25]

timvolodine@chromium.org: Please review changes in android_webview

[timvolodine, 2017-04-05 11:39:39]

On 2017/04/05 11:08:25, Shanmuga Pandi wrote:
> mailto:timvolodine@chromium.org: Please review changes in android_webview

android_webview/... -- lgtm

[foolip, 2017-04-05 15:03:26]

On 2017/04/05 10:54:42, fs wrote:
> LGTM, but you'll need someone to sign off on the webexposed bits. I'll also
> leave to them to determine how much additional red tape this will require
> (WebKit still exposes these.)

I did send an Intent to Remove for some other pointless constants in
https://groups.google.com/a/chromium.org/d/msg/blink-dev/HsAF_yFMvCM/ijFziajp...
so there's precedent for paperwork.

Use counters won't be of any use here, so what's really needed here is a look at
httparchive data. I've had a look and it looks like WEBKIT_KEYFRAME_RULE is only
used in a handful of places and not in a way that could break anything.
WEBKIT_KEYFRAMES_RULE is usually used together with KEYFRAMES_RULE, but there
are 25 cases where it's not that would be worth analyzing:

http://www.almasiran.ir/
http://www.belltravelservice.com/
http://www.biblearc.com/
http://www.boostads.net/
http://www.criminal.az/
http://www.dreamworkstv.com/
http://www.drogisterij.net/
http://www.gestordemarketing.com/
http://www.gigablue.de/
http://www.got-it.nl/
http://www.habermetre.com/
http://www.kijiji.ca/
http://www.logomakr.com/
http://www.lsm.kz/
http://www.motorclubamerica.net/
http://www.nariasoft.ir/
http://www.petgo.jp/
http://www.rafflesmedicalgroup.com/
http://www.sekisui.co.jp/
http://www.seomatik.de/
http://www.shoene-portal.jp/
http://www.simyo.nl/
http://www.thiememeulenhoff.nl/
http://www.trycaviar.com/
http://www.unibet.fr/

Shanmuga, can you analyze how (if) these sites would be affected by the removal?
With that information in hand, an Intent to Remove should easily get 3xLGTM.

[meade_UTC10, 2017-04-06 03:33:00]

meade@chromium.org changed reviewers:
+ suzyh@chromium.org

[meade_UTC10, 2017-04-06 03:33:01]

core/css lgtm. Added suzyh for LayoutTests/animations

[suzyh_UTC10 (ex-contributor), 2017-04-06 03:45:48]

On 2017/04/06 at 03:33:01, meade wrote:
> core/css lgtm. Added suzyh for LayoutTests/animations

LayoutTests/animations lgtm but repeating the requirement above for approvals
from blink-dev for the web-facing change.

alancutter@ points out that it would be useful to add a test to show that we
_don't_ support the prefixed version.

[alancutter (OOO until 2018), 2017-04-06 04:11:22]

On 2017/04/05 at 15:03:26, foolip wrote:
>
https://groups.google.com/a/chromium.org/d/msg/blink-dev/HsAF_yFMvCM/ijFziajp...

This link needs to be in the description and in the bug.

[foolip, 2017-04-06 04:39:20]

On 2017/04/06 04:11:22, alancutter wrote:
> On 2017/04/05 at 15:03:26, foolip wrote:
> >
>
https://groups.google.com/a/chromium.org/d/msg/blink-dev/HsAF_yFMvCM/ijFziajp...
> 
> This link needs to be in the description and in the bug.

That's for another removal, Event.MOUSEDOWN and other constants, that I posted
that as an example. This also needs an intent, but just as well to do the compat
analysis first.

[Shanmuga Pandi, 2017-04-07 11:08:19]

On 2017/04/05 15:03:26, foolip_UTC7 wrote:
> On 2017/04/05 10:54:42, fs wrote:
> > LGTM, but you'll need someone to sign off on the webexposed bits. I'll also
> > leave to them to determine how much additional red tape this will require
> > (WebKit still exposes these.)
> 
> I did send an Intent to Remove for some other pointless constants in
>
https://groups.google.com/a/chromium.org/d/msg/blink-dev/HsAF_yFMvCM/ijFziajp...
> so there's precedent for paperwork.
> 
> Use counters won't be of any use here, so what's really needed here is a look
at
> httparchive data. I've had a look and it looks like WEBKIT_KEYFRAME_RULE is
only
> used in a handful of places and not in a way that could break anything.
> WEBKIT_KEYFRAMES_RULE is usually used together with KEYFRAMES_RULE, but there
> are 25 cases where it's not that would be worth analyzing:
> 
> http://www.almasiran.ir/
> http://www.belltravelservice.com/
> http://www.biblearc.com/
> http://www.boostads.net/
> http://www.criminal.az/
> http://www.dreamworkstv.com/
> http://www.drogisterij.net/
> http://www.gestordemarketing.com/
> http://www.gigablue.de/
> http://www.got-it.nl/
> http://www.habermetre.com/
> http://www.kijiji.ca/
> http://www.logomakr.com/
> http://www.lsm.kz/
> http://www.motorclubamerica.net/
> http://www.nariasoft.ir/
> http://www.petgo.jp/
> http://www.rafflesmedicalgroup.com/
> http://www.sekisui.co.jp/
> http://www.seomatik.de/
> http://www.shoene-portal.jp/
> http://www.simyo.nl/
> http://www.thiememeulenhoff.nl/
> http://www.trycaviar.com/
> http://www.unibet.fr/
> 
> Shanmuga, can you analyze how (if) these sites would be affected by the
removal?
> With that information in hand, an Intent to Remove should easily get 3xLGTM.

I have loaded the each url and I could see any affect by the removal.

Below are the sites are using "rock-options/curvy-slider/js/caat.min.js"
http://www.belltravelservice.com
http://almasiran.ir
https://gestordemarketing.com
http://www.gigablue.de
http://www.motorclubamerica.net/
http://nariasoft.ir/
https://www.seomatik.de

js files are respectively
http://www.belltravelservice.com/wp-content/themes/azoomtheme/rock-options/cu...
http://almasiran.ir/wp-content/themes/azoomtheme/rock-options/curvy-slider/js...
https://gestordemarketing.com/wp-content/themes/azoomtheme/rock-options/curvy...
http://www.gigablue.de/wp-content/themes/quasartheme/rock-options/curvy-slide...
http://www.motorclubamerica.net/wp-content/themes/quasartheme/rock-options/cu...
http://nariasoft.ir/wp-content/themes/nariasoft/rock-options/curvy-slider/js/...
https://www.seomatik.de/wp-content/themes/quasartheme/rock-options/curvy-slid...


Not able to verify the below sites due to my local network issue.
http://www.boostads.net/ 
http://www.dreamworkstv.com/
http://www.drogisterij.net/
http://www.kijiji.ca/
http://www.unibet.fr/


Below sites using with constant value:
https://www.biblearc.com
https://logomakr.com/
http://www.rafflesmedicalgroup.com
https://www.trycaviar.com/
Usage is like:
        n.CSSRule.WEBKIT_KEYFRAMES_RULE = 8,
        n.CSSRule.WEBKIT_KEYFRAME_RULE = 9,


Below sites using these to add prefix "-webkit":
https://lsm.kz/
http://www.petgo.jp/
http://www.sekisui.co.jp
http://www.shoene-portal.jp
Usage is like:
   if (CSSRule.WEBKIT_KEYFRAMES_RULE) { prefix = "-webkit-"; }

In below site, the usage is commented:
http://www.habermetre.com/


Other sites are usage like "caat.min.js":
http://www.criminal.az/
http://www.got-it.nl/
https://www.simyo.nl/
https://www.thiememeulenhoff.nl/


When I checked all these sites, I haven't see any effects after removal.

[Shanmuga Pandi, 2017-04-07 11:10:07]

All these sites behaving same before and after removal of these two.

[foolip, 2017-04-07 12:02:41]

On 2017/04/07 11:10:07, Shanmuga Pandi wrote:
> All these sites behaving same before and after removal of these two.

Thanks Shanmuga, if you send an Intent to Remove and point to the above
analysis, I think you will get support for removal.

[Shanmuga Pandi, 2017-04-11 06:12:02]

Description was changed from

==========
Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

BUG=689681
==========

to

==========
Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

Intent to Remove thread:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/mW1njtgDPHA

BUG=689681
==========

[Shanmuga Pandi, 2017-04-13 12:31:29]

On 2017/04/06 03:45:48, suzyh_UTC10 wrote:
> On 2017/04/06 at 03:33:01, meade wrote:
> > core/css lgtm. Added suzyh for LayoutTests/animations
> 
> LayoutTests/animations lgtm but repeating the requirement above for approvals
> from blink-dev for the web-facing change.
> 
> alancutter@ points out that it would be useful to add a test to show that we
> _don't_ support the prefixed version.

Added the test for "_don't_ support the prefixed version" on keyframes-rule.html

[Shanmuga Pandi, 2017-04-13 12:32:09]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 12:32:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 13:49:32]

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

[commit-bot: I haz the power, 2017-04-13 13:49:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Shanmuga Pandi, 2017-04-17 09:19:11]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 09:19:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 11:41:34]

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

[commit-bot: I haz the power, 2017-04-17 11:41:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Shanmuga Pandi, 2017-04-18 06:03:27]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 06:03:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 09:02:07]

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

[commit-bot: I haz the power, 2017-04-18 09:02:08]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Shanmuga Pandi, 2017-04-18 13:59:37]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 14:00:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 17:03:35]

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

[commit-bot: I haz the power, 2017-04-18 17:03:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Shanmuga Pandi, 2017-04-20 05:57:47]

On 2017/04/18 17:03:36, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Any idea why these tests failing with this patch ?
Thanks

[Srirama, 2017-04-20 06:24:49]

On 2017/04/20 05:57:47, Shanmuga Pandi wrote:
> On 2017/04/18 17:03:36, commit-bot: I haz the power wrote:
> > Dry run: Try jobs failed on following builders:
> >   linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
> (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> 
> Any idea why these tests failing with this patch ?
> Thanks

All these tests have been disabled for official build (http://crbug.com/429294),
isn't linux_chromium_chromeos_rel_ng an official build?

[fs, 2017-04-20 08:13:44]

On 2017/04/20 at 06:24:49, srirama.m wrote:
> On 2017/04/20 05:57:47, Shanmuga Pandi wrote:
> > On 2017/04/18 17:03:36, commit-bot: I haz the power wrote:
> > > Dry run: Try jobs failed on following builders:
> > >   linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
> > (JOB_FAILED,
> > >
> >
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> > 
> > Any idea why these tests failing with this patch ?
> > Thanks
> 
> All these tests have been disabled for official build
(http://crbug.com/429294), isn't linux_chromium_chromeos_rel_ng an official
build?

The GN config used is in the generate_build_files step, and I don't see it
setting official. Maybe you're just "lucky", so keep on trying? =)

[Shanmuga Pandi, 2017-04-20 10:48:56]

The CQ bit was checked by shanmuga.m@samsung.com

[Shanmuga Pandi, 2017-04-20 10:48:57]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org,
timvolodine@chromium.org, suzyh@chromium.org, fs@opera.com
Link to the patchset: https://codereview.chromium.org/2795223002/#ps80001
(title: "Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE")

[commit-bot: I haz the power, 2017-04-20 10:49:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 10:55:02]

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

[commit-bot: I haz the power, 2017-04-20 10:55:04]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Shanmuga Pandi, 2017-04-20 11:59:12]

@foolip,

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
   
third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt

[Shanmuga Pandi, 2017-04-20 12:01:18]

On 2017/04/20 08:13:44, fs wrote:
> On 2017/04/20 at 06:24:49, srirama.m wrote:
> > On 2017/04/20 05:57:47, Shanmuga Pandi wrote:
> > > On 2017/04/18 17:03:36, commit-bot: I haz the power wrote:
> > > > Dry run: Try jobs failed on following builders:
> > > >   linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
> > > (JOB_FAILED,
> > > >
> > >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
> > > 
> > > Any idea why these tests failing with this patch ?
> > > Thanks
> > 
> > All these tests have been disabled for official build
> (http://crbug.com/429294), isn't linux_chromium_chromeos_rel_ng an official
> build?
> 
> The GN config used is in the generate_build_files step, and I don't see it
> setting official. Maybe you're just "lucky", so keep on trying? =)

I am keep trying.. :(
Any thing else I can do ? :)

[foolip, 2017-04-20 12:13:37]

On 2017/04/20 11:59:12, Shanmuga Pandi wrote:
> @foolip,
> 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for these files:
>    
>
third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt

lgtm

[Shanmuga Pandi, 2017-04-21 06:03:16]

The CQ bit was checked by shanmuga.m@samsung.com

[commit-bot: I haz the power, 2017-04-21 06:03:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 09:01:21]

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

[commit-bot: I haz the power, 2017-04-21 09:01:23]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[foolip, 2017-04-21 10:41:19]

Seems like an unusual amount of failures here, are you sure it's not actually
because of the change?

[Shanmuga Pandi, 2017-04-21 10:46:31]

On 2017/04/21 10:41:19, foolip_UTC7 wrote:
> Seems like an unusual amount of failures here, are you sure it's not actually
> because of the change?

Below are these test failing on linux_chromium_chromeos_rel_ng, I don't think
anything related with this patch.
Please let me know , the cause of these failures and solution if any.

browser_tests (with patch) browser_tests (with patch)
Run on OS: 'Ubuntu-14.04'
failures:
SortColumns/FileManagerBrowserTest.Test/1
SortColumns/FileManagerBrowserTest.Test/0
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/23
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/22
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/21
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/20
DriveSpecific/FileManagerBrowserTest.Test/6
DriveSpecific/FileManagerBrowserTest.Test/4
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/24
KeyboardOperations/FileManagerBrowserTest.Test/10
KeyboardOperations/FileManagerBrowserTest.Test/11
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/8
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/9
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/0
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/1
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/2
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/3
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/4
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/5
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/6
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/7
FileDisplay/FileManagerBrowserTest.Test/4
FileDisplay/FileManagerBrowserTest.Test/5
FileDisplay/FileManagerBrowserTest.Test/6
FileDisplay/FileManagerBrowserTest.Test/0
FileDisplay/FileManagerBrowserTest.Test/1
FileDisplay/FileManagerBrowserTest.Test/2
FileDisplay/FileManagerBrowserTest.Test/3
TabindexFocusDirectorySelected/FileManagerBrowserTestWithLegacyEventDispatch.Test/0
OpenFileDialog/FileManagerBrowserTest.Test/3
OpenFileDialog/FileManagerBrowserTest.Test/2
OpenFileDialog/FileManagerBrowserTest.Test/1
OpenFileDialog/FileManagerBrowserTest.Test/0
ExecuteDefaultTaskOnDownloads/FileManagerBrowserTest.Test/1
ExecuteDefaultTaskOnDownloads/FileManagerBrowserTest.Test/0
OpenFileDialog/FileManagerBrowserTest.Test/4
GearMenu/FileManagerBrowserTest.Test/2
GearMenu/FileManagerBrowserTest.Test/1
GearMenu/FileManagerBrowserTest.Test/0
CreateNewFolder/FileManagerBrowserTest.Test/2
CreateNewFolder/FileManagerBrowserTest.Test/3
CreateNewFolder/FileManagerBrowserTest.Test/0
CreateNewFolder/FileManagerBrowserTest.Test/1
Traverse/FileManagerBrowserTest.Test/1
Traverse/FileManagerBrowserTest.Test/0
Traverse/FileManagerBrowserTest.Test/2
TabindexFocusDownloads/FileManagerBrowserTestWithLegacyEventDispatch.Test/1
TabindexFocusDownloads/FileManagerBrowserTestWithLegacyEventDispatch.Test/0
TabindexFocus/FileManagerBrowserTestWithLegacyEventDispatch.Test/0
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/18
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/19
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/12
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/13
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/10
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/11
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/16
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/17
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/14
DirectoryTreeContextMenu/FileManagerBrowserTest.Test/15
Providers/FileManagerBrowserTest.Test/0
Providers/FileManagerBrowserTest.Test/1
Providers/FileManagerBrowserTest.Test/2
Providers/FileManagerBrowserTest.Test/3
KeyboardOperations/FileManagerBrowserTest.Test/4
KeyboardOperations/FileManagerBrowserTest.Test/5
KeyboardOperations/FileManagerBrowserTest.Test/6
KeyboardOperations/FileManagerBrowserTest.Test/7
KeyboardOperations/FileManagerBrowserTest.Test/0
KeyboardOperations/FileManagerBrowserTest.Test/1
KeyboardOperations/FileManagerBrowserTest.Test/2
KeyboardOperations/FileManagerBrowserTest.Test/3
KeyboardOperations/FileManagerBrowserTest.Test/8
KeyboardOperations/FileManagerBrowserTest.Test/9
GenericTask/FileManagerBrowserTest.Test/0
GenericTask/FileManagerBrowserTest.Test/1
ShowGridView/FileManagerBrowserTest.Test/2
ShowGridView/FileManagerBrowserTest.Test/1
ShowGridView/FileManagerBrowserTest.Test/0
RestoreGeometry/FileManagerBrowserTest.Test/2
RestoreGeometry/FileManagerBrowserTest.Test/0
RestoreGeometry/FileManagerBrowserTest.Test/1
TabIndex/FileManagerBrowserTestWithLegacyEventDispatch.Test/0
FolderShortcuts/FileManagerBrowserTest.Test/1
FolderShortcuts/FileManagerBrowserTest.Test/0
Transfer/FileManagerBrowserTest.Test/6
Transfer/FileManagerBrowserTest.Test/7
Transfer/FileManagerBrowserTest.Test/4
Transfer/FileManagerBrowserTest.Test/5
Transfer/FileManagerBrowserTest.Test/2
Transfer/FileManagerBrowserTest.Test/3
Transfer/FileManagerBrowserTest.Test/0
Transfer/FileManagerBrowserTest.Test/1
DriveSpecific/FileManagerBrowserTest.Test/2
MultiProfileFileManagerBrowserTest.BasicDownloads
DriveSpecific/FileManagerBrowserTest.Test/0
DriveSpecific/FileManagerBrowserTest.Test/1
DriveSpecific/FileManagerBrowserTest.Test/3
MultiProfileFileManagerBrowserTest.BasicDrive
DriveSpecific/FileManagerBrowserTest.Test/5
OpenAudioFiles/FileManagerBrowserTest.Test/6
OpenAudioFiles/FileManagerBrowserTest.Test/7
OpenAudioFiles/FileManagerBrowserTest.Test/4
OpenAudioFiles/FileManagerBrowserTest.Test/5
OpenAudioFiles/FileManagerBrowserTest.Test/2
OpenAudioFiles/FileManagerBrowserTest.Test/3
OpenAudioFiles/FileManagerBrowserTest.Test/0
OpenAudioFiles/FileManagerBrowserTest.Test/1
OpenAudioFiles/FileManagerBrowserTest.Test/8
DefaultTaskDialog/FileManagerBrowserTest.Test/2
DefaultTaskDialog/FileManagerBrowserTest.Test/0
DefaultTaskDialog/FileManagerBrowserTest.Test/1
Delete/FileManagerBrowserTest.Test/0
Delete/FileManagerBrowserTest.Test/1
ExecuteDefaultTaskOnDrive/FileManagerBrowserTest.Test/0

[Shanmuga Pandi, 2017-04-24 12:22:43]

On 2017/04/21 10:41:19, foolip_UTC7 wrote:
> Seems like an unusual amount of failures here, are you sure it's not actually
> because of the change?

@foolip,@fs

These failures occurs only with this patch.
To verify I had created a temp patch and failure occurs with the change in .idl
file.
https://codereview.chromium.org/2841493002/

Could you help me to find the cause and fix the failure ?

Regards,
Shanmuga

[fs, 2017-04-24 12:34:25]

On 2017/04/24 at 12:22:43, shanmuga.m wrote:
> On 2017/04/21 10:41:19, foolip_UTC7 wrote:
> > Seems like an unusual amount of failures here, are you sure it's not
actually
> > because of the change?
> 
> @foolip,@fs
> 
> These failures occurs only with this patch.
> To verify I had created a temp patch and failure occurs with the change in
.idl file.
> https://codereview.chromium.org/2841493002/
> 
> Could you help me to find the cause and fix the failure ?

ui/file_manager/file_manager/foreground/js/ui/progress_center_panel.js appears
to be referencing/using WEBKIT_KEYFRAMES_RULE. Could you try replacing that with
the non-prefixed equivalent? (I see it has a css_rule.js that defines both of
these constants, and have no idea how that all ties together, but those could
hopefully be removed too. Would need confirmation from local owners anyway.)

[Shanmuga Pandi, 2017-04-25 06:59:06]

Description was changed from

==========
Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

Intent to Remove thread:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/mW1njtgDPHA

BUG=689681
==========

to

==========
Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

Intent to Remove thread:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/mW1njtgDPHA

BUG=689681
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Shanmuga Pandi, 2017-04-25 07:00:06]

The CQ bit was checked by shanmuga.m@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-25 07:00:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Shanmuga Pandi, 2017-04-25 07:00:49]

On 2017/04/24 12:34:25, fs wrote:
> On 2017/04/24 at 12:22:43, shanmuga.m wrote:
> > On 2017/04/21 10:41:19, foolip_UTC7 wrote:
> > > Seems like an unusual amount of failures here, are you sure it's not
> actually
> > > because of the change?
> > 
> > @foolip,@fs
> > 
> > These failures occurs only with this patch.
> > To verify I had created a temp patch and failure occurs with the change in
> .idl file.
> > https://codereview.chromium.org/2841493002/
> > 
> > Could you help me to find the cause and fix the failure ?
> 
> ui/file_manager/file_manager/foreground/js/ui/progress_center_panel.js appears
> to be referencing/using WEBKIT_KEYFRAMES_RULE. Could you try replacing that
with
> the non-prefixed equivalent? (I see it has a css_rule.js that defines both of
> these constants, and have no idea how that all ties together, but those could
> hopefully be removed too. Would need confirmation from local owners anyway.)

Thanks you @fs, I have made the changes.

PTAL!

[Shanmuga Pandi, 2017-04-25 07:24:39]

shanmuga.m@samsung.com changed reviewers:
+ fukino@chromium.org

[Shanmuga Pandi, 2017-04-25 07:24:39]

fukino@chromium.org: Please review changes in ui/filemanager.

[fukino, 2017-04-25 08:22:01]

ui/file_manager lgtm. Thanks!

[commit-bot: I haz the power, 2017-04-25 08:54:17]

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

[commit-bot: I haz the power, 2017-04-25 08:54:19]

Dry run: This issue passed the CQ dry run.

[Shanmuga Pandi, 2017-04-25 08:55:23]

The CQ bit was checked by shanmuga.m@samsung.com

[Shanmuga Pandi, 2017-04-25 08:55:24]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org,
timvolodine@chromium.org, suzyh@chromium.org, foolip@chromium.org, fs@opera.com
Link to the patchset: https://codereview.chromium.org/2795223002/#ps100001
(title: "changes in ui/filemanager/")

[commit-bot: I haz the power, 2017-04-25 08:55:40]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-25 08:59:12]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493110523936410,
"parent_rev": "da89e55fac785cb364feb49d078531e987e41616", "commit_rev":
"0363d88c658806d011c31a4f48f25779e9bc5f22"}

[commit-bot: I haz the power, 2017-04-25 08:59:24]

Description was changed from

==========
Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

Intent to Remove thread:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/mW1njtgDPHA

BUG=689681
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE

Remove WEBKIT_KEYFRAMES_RULE and WEBKIT_KEYFRAME_RULE
from CSSRule interface.

Intent to Remove thread:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/mW1njtgDPHA

BUG=689681
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2795223002
Cr-Commit-Position: refs/heads/master@{#466930}
Committed:
https://chromium.googlesource.com/chromium/src/+/0363d88c658806d011c31a4f48f2...
==========

[commit-bot: I haz the power, 2017-04-25 08:59:27]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/0363d88c658806d011c31a4f48f2...