Bug 78778

Summary: [Refactoring] Use HTMLInsertionPoint instead of HTMLContentElement
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dominicc, hayato, morrita, rolandsteiner, shinyak, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 78771    
Bug Blocks: 78596    
Attachments:
Description Flags
Test
none
use InsertionPoint
none
update
none
Patch for landing none

Shinya Kawanaka
Reported 2012-02-15 22:41:41 PST
We had assumed shadow insertion point was only HTMLContentElement before. So a lot of code use HTMLContentElement* type instead of HTMLInsertionPoint type. But we now have <shadow> element, which is another insertion point. We should convert HTMLContentElement* type to HTMLInsertionPoint* in NodeRenderingContext and so on.
Attachments
Test (18.22 KB, patch)
2012-02-20 00:35 PST, Shinya Kawanaka 		no flags
DetailsFormatted DiffDiff
use InsertionPoint (12.60 KB, patch)
2012-02-20 03:12 PST, Hayato Ito 		no flags
DetailsFormatted DiffDiff
update (14.21 KB, patch)
2012-02-20 17:54 PST, Hayato Ito 		no flags
DetailsFormatted DiffDiff
Patch for landing (14.36 KB, patch)
2012-02-20 20:46 PST, Hayato Ito 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Shinya Kawanaka
Comment 1 2012-02-20 00:35:41 PST
Created attachment 127775 [details] Test
Hayato Ito
Comment 2 2012-02-20 03:12:14 PST
Created attachment 127796 [details] use InsertionPoint
Hajime Morrita
Comment 3 2012-02-20 15:57:23 PST
Comment on attachment 127796 [details] use InsertionPoint View in context: https://bugs.webkit.org/attachment.cgi?id=127796&action=review > Source/WebCore/html/shadow/HTMLContentElement.h:NaN > public: It looks we no longer need some #include. > Source/WebCore/html/shadow/InsertionPoint.cpp:43 > + , m_selections(adoptPtr(new HTMLContentSelectionList())) If we instantiate here and have a #include, there would be no reason to allocate this in heap. I guess the original class lose the point at some point.
Hayato Ito
Comment 4 2012-02-20 17:54:54 PST
Created attachment 127881 [details] update
Hayato Ito
Comment 5 2012-02-20 17:55:44 PST
(In reply to comment #3) > (From update of attachment 127796 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127796&action=review > > > Source/WebCore/html/shadow/HTMLContentElement.h:NaN > > public: > > It looks we no longer need some #include. Done. I've cleaned up #include. > > > Source/WebCore/html/shadow/InsertionPoint.cpp:43 > > + , m_selections(adoptPtr(new HTMLContentSelectionList())) > > If we instantiate here and have a #include, there would be no reason to allocate this in heap. > I guess the original class lose the point at some point. Done.
Hajime Morrita
Comment 6 2012-02-20 19:38:30 PST
Comment on attachment 127881 [details] update Let's make sure that win bot green. It looks unrelated to this patch though.
Shinya Kawanaka
Comment 7 2012-02-20 20:19:11 PST
(In reply to comment #6) > (From update of attachment 127881 [details]) > Let's make sure that win bot green. It looks unrelated to this patch though. Unfortunately, win bot is failing these days i(In reply to comment #6) > (From update of attachment 127881 [details]) > Let's make sure that win bot green. It looks unrelated to this patch though. This is an error message. 6>c:\cygwin\home\buildbot\WebKit\Source\WebCore\html\shadow\HTMLContentElement.cpp(100) : error C2819: type 'WebCore::HTMLContentSelectionList' does not have an overloaded member 'operator ->' 6> c:\cygwin\home\buildbot\webkit\source\webcore\html\shadow\HTMLContentSelector.h(77) : see declaration of 'WebCore::HTMLContentSelectionList' 6> did you intend to use '.' instead? 6>c:\cygwin\home\buildbot\WebKit\Source\WebCore\html\shadow\HTMLContentElement.cpp(100) : error C2232: '->WebCore::HTMLContentSelectionList::isEmpty' : left operand has 'class' type, use '.'
Hayato Ito
Comment 8 2012-02-20 20:46:14 PST
Created attachment 127907 [details] Patch for landing
Hayato Ito
Comment 9 2012-02-20 20:47:17 PST
This patch is related. I've fixed the debug build. (In reply to comment #6) > (From update of attachment 127881 [details]) > Let's make sure that win bot green. It looks unrelated to this patch though.
WebKit Review Bot
Comment 10 2012-02-20 22:12:47 PST
The commit-queue encountered the following flaky tests while processing attachment 127907 [details]: perf/object-keys.html bug 63769 (author: ojan@chromium.org) The commit-queue is continuing to process your patch.
WebKit Review Bot
Comment 11 2012-02-20 22:14:51 PST
Comment on attachment 127907 [details] Patch for landing Clearing flags on attachment: 127907 Committed r108303: <http://trac.webkit.org/changeset/108303>
WebKit Review Bot
Comment 12 2012-02-20 22:14:58 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.