CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule (inherited from CSSConditionRule parent rule), on getting, must return the value of media.mediaText on the rule. Setting the conditionText attribute must set the media.mediaText attribute on the rule. So conditionText of CSSConditionRule should be mutable instead of readonly, this is a TODO.

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface
https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface

BUG=651792
Committed: https://crrev.com/4aaf16a74e94cae754d8e65ddcf0bd1dafefc0ae
Cr-Commit-Position: refs/heads/master@{#425950}

[xing.xu, 2016-10-13 14:13:48]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 14:13:59]

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

[xing.xu, 2016-10-13 14:25:39]

Description was changed from

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface

BUG=651792
==========

to

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface

BUG=651792
==========

[xing.xu, 2016-10-13 14:25:39]

xing.xu@intel.com changed reviewers:
+ keishi@chromium.org, mikelawther@chromium.org

[xing.xu, 2016-10-13 14:27:19]

xing.xu@intel.com changed reviewers:
+ foolip@chromium.org

[xing.xu, 2016-10-13 15:05:38]

PTAL

[foolip, 2016-10-13 15:16:25]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 15:16:43]

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

[foolip, 2016-10-13 15:17:10]

Started a dry run, happy to leave the actual review to the others, but can
rubberstamp when needed for the webexposed test changes.

[commit-bot: I haz the power, 2016-10-13 16:29:53]

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

[commit-bot: I haz the power, 2016-10-13 16:29:54]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mike Lawther (Google), 2016-10-13 23:59:11]

mikelawther@chromium.org changed reviewers:
+ meade@chromium.org
- mikelawther@chromium.org

[xing.xu, 2016-10-14 00:37:42]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 00:38:15]

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, 2016-10-14 01:48:04]

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

[commit-bot: I haz the power, 2016-10-14 01:48:05]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xing.xu, 2016-10-14 02:15:42]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 02:16:01]

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, 2016-10-14 03:31:31]

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

[commit-bot: I haz the power, 2016-10-14 03:31:32]

Dry run: This issue passed the CQ dry run.

[xing.xu, 2016-10-14 04:50:26]

Thanks foolip@. Now the tests are fixed, PTAL.

BTW, after click trybot results's "More>>>", I found there were two
linux_chromium_rel_ng fail. One pass, one failed like this:
content_unittests (with patch) on Ubuntu-12.04 content_unittests (with patch) on
Ubuntu-12.04

4 disabled
1 flaky
failed 1

failures:
InputRouterImplTest.AckedTouchEventState
 
Does it matter?

[Timothy Loh, 2016-10-14 07:07:18]

On 2016/10/14 04:50:26, xing.xu wrote:
> Thanks foolip@. Now the tests are fixed, PTAL.
> 
> BTW, after click trybot results's "More>>>", I found there were two
> linux_chromium_rel_ng fail. One pass, one failed like this:
> content_unittests (with patch) on Ubuntu-12.04 content_unittests (with patch)
on
> Ubuntu-12.04
> 
> 4 disabled
> 1 flaky
> failed 1
> 
> failures:
> InputRouterImplTest.AckedTouchEventState
>  
> Does it matter?

Those are probably just flaky, it should be safe to ignore them.

By the way, it's helpful if you can describe in the change description what the
web-observable difference are (in this case it looks like we're adding a
conditionText attribute).

[xing.xu, 2016-10-14 08:05:10]

> By the way, it's helpful if you can describe in the change description what
the
> web-observable difference are (in this case it looks like we're adding a
> conditionText attribute).

Will update this soon.

BTW, this spec said: 
"The conditionText attribute of CSSMediaRule(defined on the CSSConditionRule
parent rule), on getting, must return the value of media.mediaText on the rule.
Setting the conditionText attribute must set the media.mediaText attribute on
the rule."

The previous patch doesn't implement the related set feature of conditionText.
Will refine this too.

[xing.xu, 2016-10-14 08:25:40]

Description was changed from

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface

BUG=651792
==========

to

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule(inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule.


Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface

BUG=651792
==========

[foolip, 2016-10-14 08:32:39]

I see no change in CSSSupportsRule.idl, did you omit that intentionally?

[foolip, 2016-10-14 08:36:44]

https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSConditionRule.idl (right):

https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSConditionRule.idl:5:
//https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface
An extra space here :)

https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSConditionRule.idl:8: readonly attribute
DOMString conditionText;
This is actually not readonly in the spec. You're just preserving behavior, but
can you add a TODO saying that it shouldn't be readonly? It could well be that
the spec is wrong, if you could investigate if it's really mutable in any engine
(whether or not they have this new interface) that'd be good. If it's not, then
a spec bug would be in order.

[xing.xu, 2016-10-14 11:58:02]

On 2016/10/14 08:36:44, foolip wrote:
>
https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/CSSConditionRule.idl (right):
> 
>
https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/CSSConditionRule.idl:5:
> //https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface
> An extra space here :)
> 
>
https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/CSSConditionRule.idl:8: readonly attribute
> DOMString conditionText;
> This is actually not readonly in the spec. You're just preserving behavior,
but
> can you add a TODO saying that it shouldn't be readonly? It could well be that
> the spec is wrong, if you could investigate if it's really mutable in any
engine
> (whether or not they have this new interface) that'd be good. If it's not,
then
> a spec bug would be in order.


You are correct. In spec, this is not readonly. Will submit a followup CL to
implement setConditionText related api.

[xing.xu, 2016-10-14 12:03:16]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 12:03:35]

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, 2016-10-14 12:05:46]

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

[commit-bot: I haz the power, 2016-10-14 12:05:47]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[xing.xu, 2016-10-14 12:25:40]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 12:25: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, 2016-10-14 12:28:08]

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

[commit-bot: I haz the power, 2016-10-14 12:28:10]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[xing.xu, 2016-10-14 13:31:29]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 13:31:45]

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, 2016-10-14 13:34:27]

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

[commit-bot: I haz the power, 2016-10-14 13:34:28]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xing.xu, 2016-10-14 22:10:09]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 22:10:33]

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, 2016-10-14 22:14:05]

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

[commit-bot: I haz the power, 2016-10-14 22:14:06]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xing.xu, 2016-10-15 04:27:02]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-15 04:27:14]

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, 2016-10-15 04:29:26]

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

[commit-bot: I haz the power, 2016-10-15 04:29:27]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[xing.xu, 2016-10-15 05:56:50]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-15 05:56:59]

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, 2016-10-15 05:59:29]

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

[commit-bot: I haz the power, 2016-10-15 05:59:30]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[xing.xu, 2016-10-15 07:44:55]

On 2016/10/14 11:58:02, xing.xu wrote:
> On 2016/10/14 08:36:44, foolip wrote:
> >
>
https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/css/CSSConditionRule.idl (right):
> > 
> >
>
https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/css/CSSConditionRule.idl:5:
> > //https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface
> > An extra space here :)
> > 
> >
>
https://codereview.chromium.org/2416893002/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/css/CSSConditionRule.idl:8: readonly
attribute
> > DOMString conditionText;
> > This is actually not readonly in the spec. You're just preserving behavior,
> but
> > can you add a TODO saying that it shouldn't be readonly? It could well be
that
> > the spec is wrong, if you could investigate if it's really mutable in any
> engine
> > (whether or not they have this new interface) that'd be good. If it's not,
> then
> > a spec bug would be in order.
> 
> 
> You are correct. In spec, this is not readonly. Will submit a followup CL to
> implement setConditionText related api.

New patch submitted, PTAL.

It is wierd all try bots failed.

[foolip, 2016-10-15 20:45:21]

On 2016/10/15 07:44:55, xing.xu wrote:
> It is wierd all try bots failed.

"Patch failure" means that the patch didn't apply cleanly, you'll need rebase on
master and upload a new patch set.

[foolip, 2016-10-15 21:02:19]

Mostly wanted to look at the IDL files, will leave the real review to the other
reviewers.

https://codereview.chromium.org/2416893002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSMediaRule.cpp (right):

https://codereview.chromium.org/2416893002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSMediaRule.cpp:53: StringBuilder result;
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface says "The
conditionText attribute (defined on the CSSConditionRule parent rule), on
getting, must return the value of media.mediaText on the rule", can it be
implemented as just media()->mediaTest(), or is MediaList not to be used
internally?

Tests for this will be needed, if there aren't any already.

https://codereview.chromium.org/2416893002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSSupportsRule.idl (right):

https://codereview.chromium.org/2416893002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSSupportsRule.idl:32: //
http://dev.w3.org/csswg/cssom/#the-cssgroupingrule-interface
These are now inherited from CSSGroupingRule and should also be removed. (So
says the removed TODO, checked that it's still true.)

[xing.xu, 2016-10-16 08:20:43]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-16 08:20:53]

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, 2016-10-16 09:17:48]

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

[commit-bot: I haz the power, 2016-10-16 09:17:49]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xing.xu, 2016-10-16 15:28:24]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-16 15:28:32]

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, 2016-10-16 16:08:32]

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

[commit-bot: I haz the power, 2016-10-16 16:08:33]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[xing.xu, 2016-10-16 22:22:26]

On 2016/10/15 21:02:19, foolip wrote:
> Mostly wanted to look at the IDL files, will leave the real review to the
other
> reviewers.
> 
>
https://codereview.chromium.org/2416893002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/CSSMediaRule.cpp (right):
> 
>
https://codereview.chromium.org/2416893002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSMediaRule.cpp:53: StringBuilder result;
> https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface says "The
> conditionText attribute (defined on the CSSConditionRule parent rule), on
> getting, must return the value of media.mediaText on the rule", can it be
> implemented as just media()->mediaTest(), or is MediaList not to be used
> internally?
> 
> Tests for this will be needed, if there aren't any already.
> 
>
https://codereview.chromium.org/2416893002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/CSSSupportsRule.idl (right):
> 
>
https://codereview.chromium.org/2416893002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSSupportsRule.idl:32: //
> http://dev.w3.org/csswg/cssom/#the-cssgroupingrule-interface
> These are now inherited from CSSGroupingRule and should also be removed. (So
> says the removed TODO, checked that it's still true.)

Done, PTAL

[meade_UTC10, 2016-10-17 02:58:44]

lgtm

Seems fine to me, but my lgtm doesn't mean much. The reviewers currently on this
CL get most approvals, but you'll need an lgtm from one of the people in
https://cs.chromium.org/chromium/src/third_party/WebKit/API_OWNERS

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/css3/media-cssom.html (right):

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/css3/media-cssom.html:3: <head>
You can omit the <html>, <head> and <body> tags.

https://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-...

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/css3/media-cssom.html:34: }, "@media inherited
from CSSConditionRule.");
Nit: It'd be nice to have newlines around the individual tests so you can
visually see they are code blocks.

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSConditionRule.cpp (right):

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSConditionRule.cpp:21:
DEFINE_TRACE(CSSConditionRule) {
So long as CSSGroupingRule has a TRACE method, you can probably omit this.

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSSupportsRule.h (right):

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSSupportsRule.h:51:
DEFINE_INLINE_VIRTUAL_TRACE() { CSSConditionRule::trace(visitor); }
If this just calls the superclass, it can be omitted.

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/StyleRule.cpp (right):

https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/StyleRule.cpp:312:
StyleRuleCondition::StyleRuleCondition(const StyleRuleCondition& o)
o -> condition
This also exists below; don't abbreviate variable names in blink
(https://www.chromium.org/blink/coding-style#TOC-Names)

[xing.xu, 2016-10-17 05:47:54]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-17 05:48:11]

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, 2016-10-17 07:26:18]

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

[commit-bot: I haz the power, 2016-10-17 07:26:19]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[xing.xu, 2016-10-17 07:44:49]

On 2016/10/17 02:58:44, Eddy wrote:
> lgtm
> 
> Seems fine to me, but my lgtm doesn't mean much. The reviewers currently on
this
> CL get most approvals, but you'll need an lgtm from one of the people in
> https://cs.chromium.org/chromium/src/third_party/WebKit/API_OWNERS
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Lay...
> File third_party/WebKit/LayoutTests/css3/media-cssom.html (right):
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Lay...
> third_party/WebKit/LayoutTests/css3/media-cssom.html:3: <head>
> You can omit the <html>, <head> and <body> tags.
> 
>
https://www.chromium.org/blink/coding-style/layout-test-style-guidelines#TOC-...
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Lay...
> third_party/WebKit/LayoutTests/css3/media-cssom.html:34: }, "@media inherited
> from CSSConditionRule.");
> Nit: It'd be nice to have newlines around the individual tests so you can
> visually see they are code blocks.
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/CSSConditionRule.cpp (right):
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSConditionRule.cpp:21:
> DEFINE_TRACE(CSSConditionRule) {
> So long as CSSGroupingRule has a TRACE method, you can probably omit this.
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/CSSSupportsRule.h (right):
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSSupportsRule.h:51:
> DEFINE_INLINE_VIRTUAL_TRACE() { CSSConditionRule::trace(visitor); }
> If this just calls the superclass, it can be omitted.
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/StyleRule.cpp (right):
> 
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/StyleRule.cpp:312:
> StyleRuleCondition::StyleRuleCondition(const StyleRuleCondition& o)
> o -> condition
> This also exists below; don't abbreviate variable names in blink
> (https://www.chromium.org/blink/coding-style#TOC-Names)

Done, PTAL.

[foolip, 2016-10-17 09:18:09]

Description was changed from

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule(inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule.


Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface

BUG=651792
==========

to

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule (inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule.


Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface

BUG=651792
==========

[foolip, 2016-10-17 09:24:02]

Thanks for working on this. Can you send an Intent to Implement and Ship for
this to blink-dev? It's a small change which will certainly get 3xLGTM, but it
does affect 3 interfaces.

[foolip, 2016-10-17 09:25:40]

https://codereview.chromium.org/2416893002/diff/160001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/css3/media-cssom.html (right):

https://codereview.chromium.org/2416893002/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/css3/media-cssom.html:29: }, "@media inherited
from CSSConditionRule.");
Can you add explicit tests for the prototype chain, e.g.
assert_equal(Object.getPrototypeOf(CSSMediaRule.prototype),
CSSConditionRule.prototype)?

[xing.xu, 2016-10-17 14:06:13]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-17 14:06:26]

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

[xing.xu, 2016-10-17 14:06:45]

On 2016/10/17 09:25:40, foolip wrote:
>
https://codereview.chromium.org/2416893002/diff/160001/third_party/WebKit/Lay...
> File third_party/WebKit/LayoutTests/css3/media-cssom.html (right):
> 
>
https://codereview.chromium.org/2416893002/diff/160001/third_party/WebKit/Lay...
> third_party/WebKit/LayoutTests/css3/media-cssom.html:29: }, "@media inherited
> from CSSConditionRule.");
> Can you add explicit tests for the prototype chain, e.g.
> assert_equal(Object.getPrototypeOf(CSSMediaRule.prototype),
> CSSConditionRule.prototype)?

Done, PTAL.

[foolip, 2016-10-17 14:23:01]

LGTM once the intent has 3xLGTM

https://codereview.chromium.org/2416893002/diff/180001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/css3/condition-cssom.html (right):

https://codereview.chromium.org/2416893002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/css3/condition-cssom.html:34: "@media screen and
(min-width: 480px) { \n" +
Oh look, a trailing whitespace. Is that part of the spec, would this test pass
on Edge and Firefox? That need not be answered in order to make this change, but
it would be good to contribute this to web-platform-tests so that WebKit can
just import it instead of writing a new test.

[xing.xu, 2016-10-17 14:47:52]

On 2016/10/17 14:23:01, foolip wrote:
> LGTM once the intent has 3xLGTM
> 
>
https://codereview.chromium.org/2416893002/diff/180001/third_party/WebKit/Lay...
> File third_party/WebKit/LayoutTests/css3/condition-cssom.html (right):
> 
>
https://codereview.chromium.org/2416893002/diff/180001/third_party/WebKit/Lay...
> third_party/WebKit/LayoutTests/css3/condition-cssom.html:34: "@media screen
and
> (min-width: 480px) { \n" +
> Oh look, a trailing whitespace. Is that part of the spec, would this test pass
> on Edge and Firefox? That need not be answered in order to make this change,
but
> it would be good to contribute this to web-platform-tests so that WebKit can
> just import it instead of writing a new test.

Yes, I also noticed this. We need to remove this trailing whitespace when
running on firefox.

It seems this is a in CSSMediaRule::cssText. How about file a bug and submit a
new patch for this?
--- a/third_party/WebKit/Source/core/css/CSSMediaRule.cpp
+++ b/third_party/WebKit/Source/core/css/CSSMediaRule.cpp
@@ -43,7 +43,7 @@ String CSSMediaRule::cssText() const {
     result.append(mediaQueries()->mediaText());
     result.append(' ');
   }
-  result.append("{ \n");
+  result.append("{\n");
   appendCSSTextForItems(result);
   result.append('}');
   return result.toString();

[xing.xu, 2016-10-17 14:54:16]

Description was changed from

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule (inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule.


Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface

BUG=651792
==========

to

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule (inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule. So conditionText of CSSConditionRule
should be mutable instead of readonly, this is a TODO.

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface
https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface

BUG=651792
==========

[foolip, 2016-10-17 14:56:27]

On 2016/10/17 14:47:52, xing.xu wrote:
> On 2016/10/17 14:23:01, foolip wrote:
> > LGTM once the intent has 3xLGTM
> > 
> >
>
https://codereview.chromium.org/2416893002/diff/180001/third_party/WebKit/Lay...
> > File third_party/WebKit/LayoutTests/css3/condition-cssom.html (right):
> > 
> >
>
https://codereview.chromium.org/2416893002/diff/180001/third_party/WebKit/Lay...
> > third_party/WebKit/LayoutTests/css3/condition-cssom.html:34: "@media screen
> and
> > (min-width: 480px) { \n" +
> > Oh look, a trailing whitespace. Is that part of the spec, would this test
pass
> > on Edge and Firefox? That need not be answered in order to make this change,
> but
> > it would be good to contribute this to web-platform-tests so that WebKit can
> > just import it instead of writing a new test.
> 
> Yes, I also noticed this. We need to remove this trailing whitespace when
> running on firefox.
> 
> It seems this is a in CSSMediaRule::cssText. How about file a bug and submit a
> new patch for this?
> --- a/third_party/WebKit/Source/core/css/CSSMediaRule.cpp
> +++ b/third_party/WebKit/Source/core/css/CSSMediaRule.cpp
> @@ -43,7 +43,7 @@ String CSSMediaRule::cssText() const {
>      result.append(mediaQueries()->mediaText());
>      result.append(' ');
>    }
> -  result.append("{ \n");
> +  result.append("{\n");
>    appendCSSTextForItems(result);
>    result.append('}');
>    return result.toString();

That would be great, can you file an issue? Since WebKit presumably has the same
bug, a test in web-platform-tests for that would be great also.

[commit-bot: I haz the power, 2016-10-17 15:21:47]

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

[commit-bot: I haz the power, 2016-10-17 15:21:48]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[xing.xu, 2016-10-18 00:37:01]

On 2016/10/17 14:56:27, foolip wrote:
> On 2016/10/17 14:47:52, xing.xu wrote:
> > On 2016/10/17 14:23:01, foolip wrote:
> > > LGTM once the intent has 3xLGTM
> > > 
> > >
> >
>
https://codereview.chromium.org/2416893002/diff/180001/third_party/WebKit/Lay...
> > > File third_party/WebKit/LayoutTests/css3/condition-cssom.html (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2416893002/diff/180001/third_party/WebKit/Lay...
> > > third_party/WebKit/LayoutTests/css3/condition-cssom.html:34: "@media
screen
> > and
> > > (min-width: 480px) { \n" +
> > > Oh look, a trailing whitespace. Is that part of the spec, would this test
> pass
> > > on Edge and Firefox? That need not be answered in order to make this
change,
> > but
> > > it would be good to contribute this to web-platform-tests so that WebKit
can
> > > just import it instead of writing a new test.
> > 
> > Yes, I also noticed this. We need to remove this trailing whitespace when
> > running on firefox.
> > 
> > It seems this is a in CSSMediaRule::cssText. How about file a bug and submit
a
> > new patch for this?
> > --- a/third_party/WebKit/Source/core/css/CSSMediaRule.cpp
> > +++ b/third_party/WebKit/Source/core/css/CSSMediaRule.cpp
> > @@ -43,7 +43,7 @@ String CSSMediaRule::cssText() const {
> >      result.append(mediaQueries()->mediaText());
> >      result.append(' ');
> >    }
> > -  result.append("{ \n");
> > +  result.append("{\n");
> >    appendCSSTextForItems(result);
> >    result.append('}');
> >    return result.toString();
> 
> That would be great, can you file an issue? Since WebKit presumably has the
same
> bug, a test in web-platform-tests for that would be great also.

Report here: https://bugs.chromium.org/p/chromium/issues/detail?id=656835

[xing.xu, 2016-10-18 04:02:55]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 04:03:12]

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, 2016-10-18 05:19:30]

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

[commit-bot: I haz the power, 2016-10-18 05:19:31]

Dry run: This issue passed the CQ dry run.

[xing.xu, 2016-10-18 05:22:53]

The Intent got 3xLGTM. PTAL.

[foolip, 2016-10-18 10:38:31]

foolip@chromium.org changed reviewers:
+ oilpan-reviews@chromium.org

[foolip, 2016-10-18 10:38:33]

Adding oilpan-reviews@ for sanity check of the tracing.

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSMediaRule.cpp (right):

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSMediaRule.cpp:55: return
mediaQueries()->mediaText();
Ah, MediaList uses this interally, makes sense.

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSMediaRule.cpp:75:
CSSGroupingRule::trace(visitor);
This should become CSSConditionRule::trace(visitor) I think.

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSSupportsRule.h (left):

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSSupportsRule.h:53:
DEFINE_INLINE_VIRTUAL_TRACE() { CSSGroupingRule::trace(visitor); }
I think this should move into CSSConditionRule instead.

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/StyleRule.cpp (right):

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/StyleRule.cpp:319:
StyleRuleMedia::StyleRuleMedia(const StyleRuleMedia& media)
It looks like the surrounding code uses |o| in these contexts, suggest sticking
with that and maybe changing them all in a CL if the owners agree.

[xing.xu, 2016-10-18 11:00:30]

On 2016/10/18 10:38:33, foolip wrote:
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSMediaRule.cpp:75:
> CSSGroupingRule::trace(visitor);
> This should become CSSConditionRule::trace(visitor) I think.
> 
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/CSSSupportsRule.h (left):
> 
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSSupportsRule.h:53:
> DEFINE_INLINE_VIRTUAL_TRACE() { CSSGroupingRule::trace(visitor); }
> I think this should move into CSSConditionRule instead.
==>About tracing:
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...


>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/StyleRule.cpp (right):
> 
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/StyleRule.cpp:319:
> StyleRuleMedia::StyleRuleMedia(const StyleRuleMedia& media)
> It looks like the surrounding code uses |o| in these contexts, suggest
sticking
> with that and maybe changing them all in a CL if the owners agree.
==> About coding style:
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...

Any idea?

[foolip, 2016-10-18 11:20:29]

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSMediaRule.cpp (right):

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSMediaRule.cpp:75:
CSSGroupingRule::trace(visitor);
On 2016/10/18 10:38:33, foolip wrote:
> This should become CSSConditionRule::trace(visitor) I think.

This still stands.

[foolip, 2016-10-18 11:21:18]

On 2016/10/18 11:00:30, xing.xu wrote:
> On 2016/10/18 10:38:33, foolip wrote:
> >
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/css/CSSMediaRule.cpp:75:
> > CSSGroupingRule::trace(visitor);
> > This should become CSSConditionRule::trace(visitor) I think.
> > 
> >
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/core/css/CSSSupportsRule.h (left):
> > 
> >
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/css/CSSSupportsRule.h:53:
> > DEFINE_INLINE_VIRTUAL_TRACE() { CSSGroupingRule::trace(visitor); }
> > I think this should move into CSSConditionRule instead.
> ==>About tracing:
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...

OK, so CSSGroupingRule.h has a DECLARE_VIRTUAL_TRACE(), then it all makes sense,
except the case just commented on.

>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/core/css/StyleRule.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/css/StyleRule.cpp:319:
> > StyleRuleMedia::StyleRuleMedia(const StyleRuleMedia& media)
> > It looks like the surrounding code uses |o| in these contexts, suggest
> sticking
> > with that and maybe changing them all in a CL if the owners agree.
> ==> About coding style:
>
https://codereview.chromium.org/2416893002/diff/140001/third_party/WebKit/Sou...
> 
> Any idea?

Sorry, didn't read Eddy's comments, please disregard.

[foolip, 2016-10-18 11:21:57]

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/CSSMediaRule.cpp (right):

https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/CSSMediaRule.cpp:75:
CSSGroupingRule::trace(visitor);
On 2016/10/18 11:20:29, foolip wrote:
> On 2016/10/18 10:38:33, foolip wrote:
> > This should become CSSConditionRule::trace(visitor) I think.
> 
> This still stands.

LGTM % this.

[xing.xu, 2016-10-18 12:20:02]

On 2016/10/18 11:21:57, foolip wrote:
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/CSSMediaRule.cpp (right):
> 
>
https://codereview.chromium.org/2416893002/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/CSSMediaRule.cpp:75:
> CSSGroupingRule::trace(visitor);
> On 2016/10/18 11:20:29, foolip wrote:
> > On 2016/10/18 10:38:33, foolip wrote:
> > > This should become CSSConditionRule::trace(visitor) I think.
> > 
> > This still stands.
> 
> LGTM % this.

Yes, thanks, done, PTAL

[xing.xu, 2016-10-18 12:20:18]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 12:20:27]

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

[foolip, 2016-10-18 12:20:51]

The CQ bit was unchecked by foolip@chromium.org

[foolip, 2016-10-18 12:21:11]

The CQ bit was checked by foolip@chromium.org

[foolip, 2016-10-18 12:21:12]

lgtm

Sending to CQ, this has been up for a while so I hope everyone has had a chance
to comment.

[foolip, 2016-10-18 12:21:13]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org
Link to the patchset: https://codereview.chromium.org/2416893002/#ps220001
(title: "Fix tracing")

[commit-bot: I haz the power, 2016-10-18 12:21:21]

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, 2016-10-18 14:14:47]

Description was changed from

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule (inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule. So conditionText of CSSConditionRule
should be mutable instead of readonly, this is a TODO.

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface
https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface

BUG=651792
==========

to

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule (inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule. So conditionText of CSSConditionRule
should be mutable instead of readonly, this is a TODO.

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface
https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface

BUG=651792
==========

[commit-bot: I haz the power, 2016-10-18 14:14:49]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-10-18 14:16:38]

Description was changed from

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule (inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule. So conditionText of CSSConditionRule
should be mutable instead of readonly, this is a TODO.

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface
https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface

BUG=651792
==========

to

==========
CSSMediaRule and CSSSupportsRule inherit from CSSConditionRule

From the spec, the conditionText attribute of CSSMediaRule (inherited from
CSSConditionRule parent rule), on getting, must return the value of
media.mediaText on the rule. Setting the conditionText attribute must set the
media.mediaText attribute on the rule. So conditionText of CSSConditionRule
should be mutable instead of readonly, this is a TODO.

Spec:
https://drafts.csswg.org/css-conditional/#the-cssmediarule-interface
https://drafts.csswg.org/css-conditional/#the-csssupportsrule-interface
https://drafts.csswg.org/css-conditional/#the-cssconditionrule-interface

BUG=651792

Committed: https://crrev.com/4aaf16a74e94cae754d8e65ddcf0bd1dafefc0ae
Cr-Commit-Position: refs/heads/master@{#425950}
==========

[commit-bot: I haz the power, 2016-10-18 14:16:41]

Patchset 12 (id:??) landed as
https://crrev.com/4aaf16a74e94cae754d8e65ddcf0bd1dafefc0ae
Cr-Commit-Position: refs/heads/master@{#425950}

[haraken, 2016-10-18 18:17:00]

Oilpan part LGTM