Bug 101484 - Clean up confused use of Document::renderer and renderView
Summary: Clean up confused use of Document::renderer and renderView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Elliott Sprehn
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-07 10:02 PST by Elliott Sprehn
Modified: 2012-11-07 15:37 PST (History)
9 users (show)

See Also:

Attachments
Patch (10.78 KB, patch)
2012-11-07 10:14 PST, Elliott Sprehn 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Elliott Sprehn 2012-11-07 10:02:20 PST 
Clean up confused use of Document::renderer and renderView
Comment 1 Elliott Sprehn 2012-11-07 10:14:09 PST 
Created attachment 172828 [details]
Patch
Comment 2 Ojan Vafai 2012-11-07 10:42:40 PST 
Comment on attachment 172828 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=172828&action=review

> Source/WebCore/dom/Element.cpp:652
> +    return document()->view()->contentsToScreen(renderer()->absoluteBoundingBoxRectIgnoringTransforms());

Don't you still need to grab the frameView()? I'm confused how this compiled.
Comment 3 Elliott Sprehn 2012-11-07 10:50:05 PST 
Comment on attachment 172828 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=172828&action=review

>> Source/WebCore/dom/Element.cpp:652
>> +    return document()->view()->contentsToScreen(renderer()->absoluteBoundingBoxRectIgnoringTransforms());
> 
> Don't you still need to grab the frameView()? I'm confused how this compiled.

document()->view() is the FrameView! :) document()->renderView() is the RenderView. Alas we have so many views.

This code was crazy before because it expanded to:

renderer()->m_node->document()->renderView()->frameView() so we went out into the render tree only to come back to ourself through m_node and then to the document and back into the render tree and then finally to the frame view... it was the rube goldberg of methods.
Comment 4 Ojan Vafai 2012-11-07 10:55:46 PST 
Comment on attachment 172828 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=172828&action=review

>>> Source/WebCore/dom/Element.cpp:652
>>> +    return document()->view()->contentsToScreen(renderer()->absoluteBoundingBoxRectIgnoringTransforms());
>> 
>> Don't you still need to grab the frameView()? I'm confused how this compiled.
> 
> document()->view() is the FrameView! :) document()->renderView() is the RenderView. Alas we have so many views.
> 
> This code was crazy before because it expanded to:
> 
> renderer()->m_node->document()->renderView()->frameView() so we went out into the render tree only to come back to ourself through m_node and then to the document and back into the render tree and then finally to the frame view... it was the rube goldberg of methods.

Oh right. I see. Lol.
Comment 5 WebKit Review Bot 2012-11-07 15:37:40 PST 
Comment on attachment 172828 [details]
Patch

Clearing flags on attachment: 172828

Committed r133813: <http://trac.webkit.org/changeset/133813>
Comment 6 WebKit Review Bot 2012-11-07 15:37:44 PST 
All reviewed patches have been landed.  Closing bug.