Bug 97953

Summary:

Kill transitive effects of SelectorChecker::checkOneSelector.

Product:

WebKit

Reporter:

Dimitri Glazkov (Google) <dglazkov>

Component:

New Bugs

Assignee:

Dimitri Glazkov (Google) <dglazkov>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cmarcelo, eric, kling, koivisto, macpherson, menard, ojan, simon.fraser, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

98181

Bug Blocks:

89879

Attachments:

Description	Flags
WIP	none
Patch	none
Patch	none

Dimitri Glazkov (Google)

Reported 2012-09-28 16:44:19 PDT

Kill transitive effects of SelectorChecker::checkOneSelector.

Attachments
WIP (8.03 KB, patch) 2012-09-28 16:44 PDT, Dimitri Glazkov (Google)	no flags	Details Formatted Diff Diff
Patch (9.85 KB, patch) 2012-10-01 14:41 PDT, Dimitri Glazkov (Google)	no flags	Details Formatted Diff Diff
Patch (9.84 KB, patch) 2012-10-01 14:43 PDT, Dimitri Glazkov (Google)	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Dimitri Glazkov (Google)

Comment 1 2012-09-28 16:44:47 PDT

Created attachment 166329 [details] WIP

Eric Seidel (no email)

Comment 2 2012-09-28 18:00:14 PDT

Nice!

Dimitri Glazkov (Google)

Comment 3 2012-10-01 14:41:17 PDT

Created attachment 166546 [details] Patch

Dimitri Glazkov (Google)

Comment 4 2012-10-01 14:43:27 PDT

Created attachment 166548 [details] Patch

Eric Seidel (no email)

Comment 5 2012-10-01 14:47:24 PDT

Comment on attachment 166548 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166548&action=review > Source/WebCore/css/SelectorChecker.cpp:448 > + if (context.selector->m_match == CSSSelector::PseudoElement) { None of the other callers care about this block?

Dimitri Glazkov (Google)

Comment 6 2012-10-01 14:49:49 PDT

Comment on attachment 166548 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166548&action=review >> Source/WebCore/css/SelectorChecker.cpp:448 >> + if (context.selector->m_match == CSSSelector::PseudoElement) { > > None of the other callers care about this block? Well, no -- since the code inside is operating on the assumption that it's looking at a pseudoelement selector :)

Eric Seidel (no email)

Comment 7 2012-10-01 15:33:51 PDT

Comment on attachment 166548 [details] Patch LGTM.

WebKit Review Bot

Comment 8 2012-10-01 15:54:16 PDT

Comment on attachment 166548 [details] Patch Clearing flags on attachment: 166548 Committed r130089: <http://trac.webkit.org/changeset/130089>

WebKit Review Bot

Comment 9 2012-10-01 15:54:19 PDT

All reviewed patches have been landed. Closing bug.

Ojan Vafai

Comment 10 2012-10-02 11:13:14 PDT

This may have caused a performance regression on the dromaeo tests. http://build.chromium.org/f/chromium/perf/linux-release-webkit-latest/dromaeo_domcoremodify/report.html?history=150&rev=-1&graph=dom_modify http://build.chromium.org/f/chromium/perf/linux-release-webkit-latest/dom_perf/report.html?history=150&rev=-1&graph=CloneNodes http://build.chromium.org/f/chromium/perf/linux-release-webkit-latest/dromaeo_domcoremodify/report.html?history=150&rev=-1&graph=dom_modify_insertBefore http://trac.webkit.org/log/?verbose=on&rev=130099&stop_rev=130089

WebKit Review Bot

Comment 11 2012-10-02 11:34:42 PDT

Re-opened since this is blocked by bug 98181

Note You need to log in before you can comment on or make changes to this bug.