Bug 118873 - Use explicit HTMLFrameElementBase cast and introduce toHTMLFrameElementBase
Summary: Use explicit HTMLFrameElementBase cast and introduce toHTMLFrameElementBase
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kangil Han
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-18 17:21 PDT by Kangil Han
Modified: 2013-07-19 02:09 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.79 KB, patch)
2013-07-18 17:30 PDT, Kangil Han
no flags Details | Formatted Diff | Diff
Patch (9.49 KB, patch)
2013-07-18 18:32 PDT, Kangil Han
no flags Details | Formatted Diff | Diff
Patch (9.57 KB, patch)
2013-07-18 22:21 PDT, Kangil Han
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kangil Han 2013-07-18 17:21:26 PDT
Use explicit HTMLFrameElementBase cast
Comment 1 Kangil Han 2013-07-18 17:30:43 PDT
Created attachment 207039 [details]
Patch
Comment 2 Ryosuke Niwa 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?
Comment 3 Ryosuke Niwa 2013-07-18 18:00:15 PDT
I also think it'll be valuable to add toHTMLFrameElementBase calls in the same patch.
Comment 4 Kangil Han 2013-07-18 18:32:08 PDT
Created attachment 207045 [details]
Patch
Comment 5 Kangil Han 2013-07-18 18:34:24 PDT
Took Ryosuke's comment into consideration.
Comment 6 Ryosuke Niwa 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.
Comment 7 Ryosuke Niwa 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.
Comment 8 Kangil Han 2013-07-18 22:21:01 PDT
Created attachment 207058 [details]
Patch
Comment 9 Kangil Han 2013-07-18 22:23:10 PDT
Took Ryosuke's comment into consideration.
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2013-07-19 02:09:22 PDT
All reviewed patches have been landed.  Closing bug.