Bug 43293

Summary: HTMLStyleElement/SVGStyleElement need to share more code
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: WebCore Misc.Assignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, gustavo, krit, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch v2
none
Patch v3 krit: review+, krit: commit-queue-

Description Nikolas Zimmermann 2010-07-31 01:05:51 PDT 
HTMLStyleElement and SVGStyleElement share the StyleElement base class. Some years ago the code was perfectly shared between those two classes.
This has been changed since a while, and HTMLStyleElement contains functionality SVGStyleElement lacks (line number reporting for style sheets etc..)

Refactor the code, to save virtual function calls, and to make both classes simpler.
Comment 1 Nikolas Zimmermann 2010-07-31 01:11:17 PDT 
Created attachment 63141 [details]
Patch
Comment 2 Early Warning System Bot 2010-07-31 01:19:37 PDT 
Attachment 63141 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3646082
Comment 3 Eric Seidel (no email) 2010-07-31 01:22:05 PDT 
Attachment 63141 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3648073
Comment 4 Nikolas Zimmermann 2010-07-31 01:22:49 PDT 
Created attachment 63142 [details]
Patch v2

Oops, a last minute change broke the build, restore m_sheet as protected variable.
Comment 5 Eric Seidel (no email) 2010-07-31 01:28:01 PDT 
Attachment 63142 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3576716
Comment 6 Early Warning System Bot 2010-07-31 01:28:22 PDT 
Attachment 63142 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3592647
Comment 7 Nikolas Zimmermann 2010-07-31 01:31:56 PDT 
Created attachment 63143 [details]
Patch v3

*sigh* uploaded wrong version.
Comment 8 WebKit Review Bot 2010-07-31 01:37:17 PDT 
Attachment 63141 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3635170
Comment 9 WebKit Review Bot 2010-07-31 01:40:05 PDT 
Attachment 63141 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3640128
Comment 10 WebKit Review Bot 2010-07-31 01:43:26 PDT 
Attachment 63141 [details] did not build on win:
Build output: http://queues.webkit.org/results/3559720
Comment 11 Dirk Schulze 2010-07-31 10:17:28 PDT 
Comment on attachment 63143 [details]
Patch v3

LGTM. Can you please add ASSERT's for document in StyleElement::insertedIntoDocument, removedFromDocument and sheetLoaded?
Comment 12 Nikolas Zimmermann 2010-07-31 10:31:25 PDT 
Thanks, also added ASSERT(element) while I was at it. Landed in r64420.