|Summary:||Remove the SVG instance tree|
|Product:||WebKit||Reporter:||Philip Rogers <pdr>|
|Component:||SVG||Assignee:||Darin Adler <darin>|
|Severity:||Normal||CC:||andersca, cdumez, darin, dino, jeffrey+webkit, kling, rniwa, sabouhallawa, sam, thorton, zimmermann|
|Version:||528+ (Nightly build)|
|Bug Depends on:||140679, 140826, 140875, 140984, 141025, 141108, 141148, 141374|
Description Philip Rogers 2015-01-18 17:58:56 PST
Supporting the instance tree requires a lot of complexity and it can probably be removed without affecting any users. The instance tree was ahead of it's time (many of the same issues are on public-webapps right now!) but it never caught on with users. Chromium's usecounters found no usage of instanceRoot and it was removed entirely in mid-2014. I'm not aware of any web compatibility fallout from removing it (not a single bug, AFAIK). There is support in the SVGWG for removing the instance tree in SVG2. The instance tree was never implemented in Gecko. Removing the instance tree in blink took a few patches but most were mechanical. All the patches are listed on crbug.com/313438 and would be pretty easy to port to WebKit.  https://groups.google.com/a/chromium.org/d/msg/blink-dev/mgYCJuzfU0Q/8jdhjY0pNgoJ  https://lists.w3.org/Archives/Public/www-svg/2014Jan/0014.html
Comment 1 Darin Adler 2015-01-19 13:31:10 PST
Sounds good to me.
Comment 2 Sam Weinig 2015-01-19 17:34:50 PST
I'm fine with this as well.
Comment 3 Darin Adler 2015-01-20 08:21:26 PST
I’m going to take a crack at this.
Comment 4 Darin Adler 2015-01-20 08:40:31 PST
Investigating crbug.com/313438 further, it’s clear this is a pretty big project that Rob Buis did! But we can indeed take advantage of those patches and redo this for WebKit; it will be 10 or so patches to do it all. I am about to land a first patch.
Comment 6 Darin Adler 2015-02-08 17:13:40 PST
Committed r179810: <http://trac.webkit.org/changeset/179810>
Comment 7 Chris Dumez 2015-02-09 10:12:02 PST
This may have caused a perf regression on the HTML5 full-render test: - https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-mavericks%22%2C%22Parser%2Fhtml5-full-render%3ATime%22%5D%5D (Mavericks / 3.6% regression) - https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22efl%22%2C%22Parser%2Fhtml5-full-render%3ATime%22%5D%5D (EFL / 2.3% regression) I am not 100% sure it is caused by this change but the range on the EFL bot is small enough that I'd say it is very likely.
Comment 8 Chris Dumez 2015-02-09 10:18:03 PST
(In reply to comment #7) > This may have caused a perf regression on the HTML5 full-render test: > - > https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac- > mavericks%22%2C%22Parser%2Fhtml5-full-render%3ATime%22%5D%5D (Mavericks / > 3.6% regression) > - > https://perf.webkit.org/ > #mode=charts&chartList=%5B%5B%22efl%22%2C%22Parser%2Fhtml5-full- > render%3ATime%22%5D%5D (EFL / 2.3% regression) > > I am not 100% sure it is caused by this change but the range on the EFL bot > is small enough that I'd say it is very likely. Looking at the change though, I really don't see what could have caused a regression. I think Ryosuke is planning to confirm the regression locally (may be a flake on the bot or another commit).
Comment 9 Darin Adler 2015-02-09 14:05:48 PST
Are there any SVG <use> elements in the full-render test?