Bug 135200 - compile window-inactive and fullscreen pseudo classes
Summary: compile window-inactive and fullscreen pseudo classes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-23 09:21 PDT by Alex Christensen
Modified: 2014-07-23 13:20 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.88 KB, patch)
2014-07-23 09:23 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (9.44 KB, patch)
2014-07-23 10:12 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (9.45 KB, patch)
2014-07-23 10:13 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (9.36 KB, patch)
2014-07-23 10:38 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (9.00 KB, patch)
2014-07-23 11:12 PDT, Alex Christensen
benjamin: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (482.83 KB, application/zip)
2014-07-23 12:17 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2014-07-23 09:21:58 PDT
I fixed a bug with window-inactive yesterday, so why not compile it, too?
Comment 1 Alex Christensen 2014-07-23 09:23:42 PDT
Created attachment 235353 [details]
Patch
Comment 2 Alex Christensen 2014-07-23 10:04:59 PDT
I was on a roll and compiled the fullscreen pseudo classes, too.
Comment 3 Alex Christensen 2014-07-23 10:12:23 PDT
Created attachment 235355 [details]
Patch
Comment 4 Alex Christensen 2014-07-23 10:13:29 PDT
Created attachment 235356 [details]
Patch
Comment 5 Alex Christensen 2014-07-23 10:38:58 PDT
Created attachment 235358 [details]
Patch
Comment 6 Alex Christensen 2014-07-23 10:40:51 PDT
https://i.imgflip.com/ajjz5.jpg
Comment 7 Benjamin Poulain 2014-07-23 11:05:37 PDT
Comment on attachment 235358 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235358&action=review

Awesome!

r- because I have serious doubt about PseudoClassAnyLink, the patch looks good otherwise.

> Source/WebCore/css/SelectorChecker.cpp:484
> +        } else if (selector->pseudoClassType() == CSSSelector::PseudoClassWindowInactive)
> +            return isWindowInactive(element);

Can't you juste remove this code? This is handled properly by the switch() below.

> Source/WebCore/cssjit/SelectorCompiler.cpp:409
> +            return FunctionType::SimpleSelectorChecker;

Indentation issue here.

> Source/WebCore/cssjit/SelectorCompiler.cpp:415
> +            return FunctionType::SimpleSelectorChecker;

ditto.

> Source/WebCore/cssjit/SelectorCompiler.cpp:-417
> -        fragment.pseudoClasses.add(CSSSelector::PseudoClassLink);
> -        return FunctionType::SimpleSelectorChecker;
> -

This does not look right. There is no handler for PseudoClassAnyLink, it is aliased with the handler of PseudoClassLink.

> Source/WebCore/cssjit/SelectorCompiler.cpp:573
> +        return FunctionType::CannotCompile;

I think CannotMatchAnything would make sense.
Comment 8 Alex Christensen 2014-07-23 11:12:28 PDT
Created attachment 235361 [details]
Patch
Comment 9 Build Bot 2014-07-23 12:17:00 PDT
Comment on attachment 235361 [details]
Patch

Attachment 235361 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6743218150440960

New failing tests:
media/track/add-and-remove-track.html
Comment 10 Build Bot 2014-07-23 12:17:02 PDT
Created attachment 235366 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 11 Alex Christensen 2014-07-23 13:20:59 PDT
test failure probably not from this change.

https://trac.webkit.org/r171486