matchMedia('(display-mode: <notValid>)').matches should not evaluate true

matchMedia('(display-mode: <notValid>)').matches should not evaluate true

Make matchMedia('(display-mode: <not_valid_input>)').matches to fail.
It should return always true with no input and if conditions met
with legal inputs. If MediaQueryExpValue is not valid, then
there is no input and return true. After that if no ID,
input is invalid and return false.

BUG=471703
Committed: https://crrev.com/b8ff10300d8a3eed9a7e7713c2ef0606d95a42f2
Cr-Commit-Position: refs/heads/master@{#353988}

[Mikhail, 2015-10-13 14:01:16]

mikhail.pozdnyakov@intel.com changed reviewers:
+ mikhail.pozdnyakov@intel.com

[Mikhail, 2015-10-13 14:01:16]

https://codereview.chromium.org/1395543005/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/fast/media/mq-display-mode.html (right):

https://codereview.chromium.org/1395543005/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/media/mq-display-mode.html:23:
"(display-mode: fine)",
comment here, like "// Unexpected values."

https://codereview.chromium.org/1395543005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp (right):

https://codereview.chromium.org/1395543005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp:224: if
(!value.isValid())
would be nice to have a short comment here explaining what that is for

[Olli Raula, 2015-10-13 14:10:53]

Comments added, thx

[Mikhail, 2015-10-13 14:19:42]

non-owner lgtm

[Yoav Weiss, 2015-10-13 14:24:46]

yoav@yoav.ws changed reviewers:
+ yoav@yoav.ws

[Yoav Weiss, 2015-10-13 14:24:47]

https://codereview.chromium.org/1395543005/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/media/mq-display-mode-expected.txt
(right):

https://codereview.chromium.org/1395543005/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/media/mq-display-mode-expected.txt:9: Query
"(display-mode: fine)": false
Does this test what the `id(!isValid) return true;` path?

[Olli Raula, 2015-10-13 14:36:00]

>
https://codereview.chromium.org/1395543005/diff/20001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/fast/media/mq-display-mode-expected.txt:9:
Query
> "(display-mode: fine)": false
> Does this test what the `id(!isValid) return true;` path?

No, those test:  if (!value.isID) return false;
First line("(display-mode)",) tests: id(!isValid) return true;

[kenneth.christiansen, 2015-10-14 06:25:49]

On 2015/10/13 14:36:00, Olli Raula wrote:
> >
>
https://codereview.chromium.org/1395543005/diff/20001/third_party/WebKit/Layo...
> > third_party/WebKit/LayoutTests/fast/media/mq-display-mode-expected.txt:9:
> Query
> > "(display-mode: fine)": false
> > Does this test what the `id(!isValid) return true;` path?
> 
> No, those test:  if (!value.isID) return false;
> First line("(display-mode)",) tests: id(!isValid) return true;

But could it test it, or is it already being tested?

[Olli Raula, 2015-10-14 06:48:48]

On 2015/10/14 06:25:49, kenneth.christiansen wrote:
> On 2015/10/13 14:36:00, Olli Raula wrote:
> > >
> >
>
https://codereview.chromium.org/1395543005/diff/20001/third_party/WebKit/Layo...
> > > third_party/WebKit/LayoutTests/fast/media/mq-display-mode-expected.txt:9:
> > Query
> > > "(display-mode: fine)": false
> > > Does this test what the `id(!isValid) return true;` path?
> > 
> > No, those test:  if (!value.isID) return false;
> > First line("(display-mode)",) tests: id(!isValid) return true;
> 
> But could it test it, or is it already being tested?

Hmm, I clarify it more. Those false inputs where the actual question was not
test "id(!isValid) return true;" path. They test " if (!value.isID) return
false" path. The first line of those testst: "(display-mode)" test the
id(!isValid) return true;` path. So the thing was that `id(!isValid) return
true;` path is tested but not there where the question was.

[kenneth.r.christiansen, 2015-10-14 08:33:58]

kenneth.r.christiansen@intel.com changed reviewers:
+ kenneth.r.christiansen@intel.com

[kenneth.r.christiansen, 2015-10-14 08:33:58]

lgtm

[kenneth.r.christiansen, 2015-10-14 08:34:03]

The CQ bit was checked by kenneth.r.christiansen@intel.com

[commit-bot: I haz the power, 2015-10-14 08:35:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1395543005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1395543005/20001

[commit-bot: I haz the power, 2015-10-14 09:55:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-14 09:56:50]

Patchset 2 (id:??) landed as
https://crrev.com/b8ff10300d8a3eed9a7e7713c2ef0606d95a42f2
Cr-Commit-Position: refs/heads/master@{#353988}

[Yoav Weiss, 2015-10-14 10:53:02]

LGTM