Bug 101173

Summary: Shadow DOM should be able to be disabled per context.
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: DOMAssignee: Hajime Morrita <morrita>
Status: RESOLVED WONTFIX    
Severity: Normal CC: abarth, dglazkov, morrita, ojan, shinyak, webcomponents-bugzilla, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 94082    
Attachments:
Description Flags
Patch none

Hajime Morrita
Reported 2012-11-04 18:19:24 PST
Basically this disables reverts http://trac.webkit.org/changeset/131549 because Chromium wants to temporally disable it.
Attachments
Patch (11.50 KB, patch)
2012-11-04 18:29 PST, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2012-11-04 18:29:01 PST
Created attachment 172254 [details] Patch
WebKit Review Bot
Comment 2 2012-11-04 19:10:11 PST
Comment on attachment 172254 [details] Patch Clearing flags on attachment: 172254 Committed r133429: <http://trac.webkit.org/changeset/133429>
WebKit Review Bot
Comment 3 2012-11-04 19:10:15 PST
All reviewed patches have been landed. Closing bug.
Ojan Vafai
Comment 4 2012-11-07 22:16:31 PST
This patch looks like it caused a bunch of perf/memory regressions: http://build.chromium.org/f/chromium/perf/mac-release-10.6-webkit-latest/dom_perf/report.html?rev=166198&graph=CreateNodes&history=100 http://build.chromium.org/f/chromium/perf/mac-release-10.6-webkit-latest/intl1/report.html?rev=166182&graph=V8.MemoryHeapSampleTotalUsed_0.75&trace=V8.MemoryHeapSampleTotalUsed_0.75&history=150 http://build.chromium.org/f/chromium/perf/chromium-rel-win7-webkit/intl2/report.html?rev=165925&graph=ws_single_peak_r&history=100 The only other patch in the regression range was rolled out in https://bugs.webkit.org/show_bug.cgi?id=101533 and the regression didn't go away. It's hard for me to see where the regression would come from. The only thing I can think of is maybe the implementation of V8EnabledPerContext versus V8EnabledAtRuntime? That's just a wild guess. Is it possible at this point to do a rollback so we can be 100% sure it was this patch?
Shinya Kawanaka
Comment 5 2012-11-07 22:20:40 PST
Can we think that regression comes from chromium side instead of WebKit side? This is also a wild guess, though.
Ojan Vafai
Comment 6 2012-11-07 22:28:42 PST
(In reply to comment #5) > Can we think that regression comes from chromium side instead of WebKit side? > This is also a wild guess, though. I considered that, but on the non-webkit canary bots, the same regression happened when this webkit revision was pulled into chromium deps and not at the same chromium revisions as on the webkit canary bots. So, I don't see how it could be a chromium side change. It's certainly possible I got confused and misread the graphs though. Feel free to double-check my conclusions. :)
Shinya Kawanaka
Comment 7 2012-11-08 01:48:44 PST
Hmm... I would like to hear morrita's comment.
Dimitri Glazkov (Google)
Comment 8 2012-11-08 09:01:10 PST
Reverted r133429 for reason: Speculative roll out, investigating perf regression. Committed r133904: <http://trac.webkit.org/changeset/133904>
Ojan Vafai
Comment 9 2012-11-08 11:10:39 PST
Looks like this was the cause of the regression. All the graphs listed have recovered their losses.
Hajime Morrita
Comment 10 2012-11-26 02:03:30 PST
This was rolled out at r133904 and we have no plan to land this again. Closing.
Note You need to log in before you can comment on or make changes to this bug.