[Webkit-unassigned] [Bug 52449] Crash when logging into gmail.com with frame flattening turned on.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52449


Antti Koivisto <koivisto at iki.fi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #81666|review?                     |review-
               Flag|                            |




--- Comment #33 from Antti Koivisto <koivisto at iki.fi>  2011-02-08 11:46:41 PST ---
(From update of attachment 81666)
View in context: https://bugs.webkit.org/attachment.cgi?id=81666&action=review

The patch is basically ok but  I still say r- as I'd like to see a round of cleanups.

> Source/WebCore/page/FrameView.cpp:728
> +    if (m_frame->settings() && m_frame->settings()->frameFlatteningEnabled()) {

Please make a variable for  "parent() && m_frame->settings() && m_frame->settings()->frameFlatteningEnabled()" and call it "inSubframeLayoutWithFrameFlattening" or something. You can use it here and later...

> Source/WebCore/page/FrameView.cpp:730
> +        FrameView* parentView = static_cast<FrameView*>(parent());
> +        if (parentView && !parentView->m_nestedLayoutCount) {

You shouldn't just cast without testing the type (even if it currently might work in practice). See other places in the file for examples on how to get the parent FrameView without casting by using tree().

> Source/WebCore/page/FrameView.cpp:770
> +    if (!m_nestedLayoutCount && !m_inSynchronousPostLayout && m_hasPendingPostLayoutTasks && (!(parent() && m_frame->settings() && m_frame->settings()->frameFlatteningEnabled()))) {

...here...

> Source/WebCore/page/FrameView.cpp:947
> +        bool deferPostLayoutTasks = parent() && m_frame->settings() && m_frame->settings()->frameFlatteningEnabled();
> +        if (!m_inSynchronousPostLayout && !deferPostLayoutTasks) {

and here...

>> Source/WebCore/page/FrameView.cpp:954
>> +        if (!m_hasPendingPostLayoutTasks && ((needsLayout() || m_inSynchronousPostLayout) ||  deferPostLayoutTasks)) {
> 
> Extra space here.

and here. Also remove the extra space and remove the inner parenthesis from the or condition, they do nothing.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.