Bug 80090 - Move the 'overflow' event dispatching logic out of RenderLayer
Summary: Move the 'overflow' event dispatching logic out of RenderLayer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Julien Chaffraix
URL:
Keywords:
Depends on:
Blocks: 75568
  Show dependency treegraph
 
Reported: 2012-03-01 20:19 PST by Julien Chaffraix
Modified: 2012-03-02 11:43 PST (History)
4 users (show)

See Also:


Attachments
Proposed refactoring: add an RAII class to handle dispatching events. (14.08 KB, patch)
2012-03-01 20:38 PST, Julien Chaffraix
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 2012-03-01 20:19:22 PST
Currently RenderLayer keeps track of whether we overflow in order to properly dispatch an 'overflow' event. Unfortunately this is not needed as RenderBox already knows whether it overflows (as it tracks its layout overflow) thus we could move the logic there. This would remove some unneeded logic from RenderLayer so that we can skip having them around.
Comment 1 Julien Chaffraix 2012-03-01 20:38:21 PST
Created attachment 129801 [details]
Proposed refactoring: add an RAII class to handle dispatching events.
Comment 2 Simon Fraser (smfr) 2012-03-01 20:44:48 PST
Comment on attachment 129801 [details]
Proposed refactoring: add an RAII class to handle dispatching events.

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

> Source/WebCore/rendering/RenderBlock.cpp:126
> +    void computeOverflowStatus(bool& hasHorizontalLayoutOverflow, bool& hasVerticalLayoutOverflow)
> +    {
> +        hasHorizontalLayoutOverflow = m_block->hasHorizontalLayoutOverflow();
> +        hasVerticalLayoutOverflow = m_block->hasVerticalLayoutOverflow();
> +    }

Having this method seems like overkill, since it just retrieves two bools.
Comment 3 Julien Chaffraix 2012-03-02 10:50:27 PST
Comment on attachment 129801 [details]
Proposed refactoring: add an RAII class to handle dispatching events.

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

FYI will also fix the bug mentioned by Stephen White in https://bugs.webkit.org/show_bug.cgi?id=52064#c3 as we now share the same logic as overflow-events.html

>> Source/WebCore/rendering/RenderBlock.cpp:126
>> +    }
> 
> Having this method seems like overkill, since it just retrieves two bools.

Makes sense. Will be removed.
Comment 4 Julien Chaffraix 2012-03-02 11:24:36 PST
Committed r109587: <http://trac.webkit.org/changeset/109587>
Comment 5 Andy Estes 2012-03-02 11:37:27 PST
Looks like this broke the build (at least on Lion): http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Build%29/builds/39454/steps/compile-webkit/logs/stdio


/Users/estes/Safari/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:123:38: error: unused parameter 'hasHorizontalLayoutOverflow' [-Werror,-Wunused-parameter,2]
     void computeOverflowStatus(bool& hasHorizontalLayoutOverflow, bool& hasVerticalLayoutOverflow)
                                      ^
/Users/estes/Safari/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:123:73: error: unused parameter 'hasVerticalLayoutOverflow' [-Werror,-Wunused-parameter,2]
     void computeOverflowStatus(bool& hasHorizontalLayoutOverflow, bool& hasVerticalLayoutOverflow)
                                                                         ^
2 errors generated.
Comment 6 Julien Chaffraix 2012-03-02 11:43:53 PST
> /Users/estes/Safari/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:123:38: error: unused parameter 'hasHorizontalLayoutOverflow' [-Werror,-Wunused-parameter,2]
>      void computeOverflowStatus(bool& hasHorizontalLayoutOverflow, bool& hasVerticalLayoutOverflow)
>                                       ^
> /Users/estes/Safari/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:123:73: error: unused parameter 'hasVerticalLayoutOverflow' [-Werror,-Wunused-parameter,2]
>      void computeOverflowStatus(bool& hasHorizontalLayoutOverflow, bool& hasVerticalLayoutOverflow)
>                                                                          ^
> 2 errors generated.

Apologies Andy, the build fix is committed in http://trac.webkit.org/changeset/109589. Let me know if you still see a build failure.