Bug 118873

Summary: Use explicit HTMLFrameElementBase cast and introduce toHTMLFrameElementBase
Product: WebKit Reporter: Kangil Han <kangil.han>
Component: WebCore Misc.Assignee: Kangil Han <kangil.han>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, esprehn+autocc, glenn, kling, koivisto, rniwa, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Kangil Han
Reported 2013-07-18 17:21:26 PDT
Use explicit HTMLFrameElementBase cast
Attachments
Patch (2.79 KB, patch)
2013-07-18 17:30 PDT, Kangil Han 		no flags
DetailsFormatted DiffDiff
Patch (9.49 KB, patch)
2013-07-18 18:32 PDT, Kangil Han 		no flags
DetailsFormatted DiffDiff
Patch (9.57 KB, patch)
2013-07-18 22:21 PDT, Kangil Han 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Kangil Han
Comment 1 2013-07-18 17:30:43 PDT
Created attachment 207039 [details] Patch
Ryosuke Niwa
Comment 2 2013-07-18 17:59:48 PDT
Comment on attachment 207039 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207039&action=review > Source/WebCore/ChangeLog:9 > + It should be HTMLFrameElementBase that embraces both HTMLFrameElement and HTMLIFrameElement. > + This also makes correct toFooElement possible. Can we also fix RenderLayer::scrollRectToVisible not to cast the pointer to HTMLFrameElement?
Ryosuke Niwa
Comment 3 2013-07-18 18:00:15 PDT
I also think it'll be valuable to add toHTMLFrameElementBase calls in the same patch.
Kangil Han
Comment 4 2013-07-18 18:32:08 PDT
Created attachment 207045 [details] Patch
Kangil Han
Comment 5 2013-07-18 18:34:24 PDT
Took Ryosuke's comment into consideration.
Ryosuke Niwa
Comment 6 2013-07-18 20:36:53 PDT
Comment on attachment 207045 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207045&action=review > Source/WebCore/rendering/RenderLayer.cpp:2267 > -static inline bool frameElementAndViewPermitScroll(HTMLFrameElement* frameElement, FrameView* frameView) > +static inline bool frameElementAndViewPermitScroll(HTMLFrameElementBase* frameElementBase, FrameView* frameView) What I meant to say is that we should fix the caller of this function where we're incorrectly casting HTMLIFrameElement to HTMLFrameElement.
Ryosuke Niwa
Comment 7 2013-07-18 21:47:23 PDT
Comment on attachment 207045 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207045&action=review > Source/WebCore/rendering/RenderFrameBase.cpp:76 > - HTMLFrameElementBase* element = static_cast<HTMLFrameElementBase*>(node()); > + HTMLFrameElementBase* element = toHTMLFrameElementBase(node()); > bool isScrollable = element->scrollingMode() != ScrollbarAlwaysOff; It seems like this local variable is used only once. You might as well as do: toHTMLFrameElementBase(node())->scrollingMode() != ScrollbarAlwaysOff instead.
Kangil Han
Comment 8 2013-07-18 22:21:01 PDT
Created attachment 207058 [details] Patch
Kangil Han
Comment 9 2013-07-18 22:23:10 PDT
Took Ryosuke's comment into consideration.
WebKit Commit Bot
Comment 10 2013-07-19 02:09:18 PDT
Comment on attachment 207058 [details] Patch Clearing flags on attachment: 207058 Committed r152889: <http://trac.webkit.org/changeset/152889>
WebKit Commit Bot
Comment 11 2013-07-19 02:09:22 PDT
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.