[Webkit-unassigned] [Bug 121284] Settings keeps a dangling pointer to m_page after m_page has been deleted

[bugzilla-daemon at webkit.org]

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





--- Comment #1 from Darin Adler <darin at apple.com>  2013-09-13 10:09:04 PST ---
(From update of attachment 211525)
View in context: https://bugs.webkit.org/attachment.cgi?id=211525&action=review

This patch looks great to me. What’s missing is a change log, some small stylistic changes, and either a regression test or a reason we can’t make a regression test.

> Source/WebCore/page/Settings.cpp:55
> +    if(page)

Need to put a space after "if". Probably best to use an early return here instead. Or changes this to take a Page& and put the null check at the caller.

> Source/WebCore/page/Settings.cpp:308
> +    if(m_page)

Space after "if".

> Source/WebCore/page/Settings.cpp:327
> +    if(m_page) {

Space after if.

> Source/WebCore/page/Settings.cpp:346
> +      FrameView* view = m_page->mainFrame().view();
> +      ASSERT(view);

Indentation needs to be four spaces, not two.

> Source/WebCore/page/Settings.cpp:375
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:412
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:423
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:446
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:452
> +    return m_page ? m_page->minimumTimerInterval() : 0;

Not sure that 0 is a safe value to return.

> Source/WebCore/page/Settings.cpp:467
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:473
> +    return m_page ? m_page->timerAlignmentInterval() : 0;

Not sure that 0 is a safe value to return.

> Source/WebCore/page/Settings.cpp:495
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:504
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:526
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:551
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:559
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:635
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:646
> +    if(m_page)

Space after if.

> Source/WebCore/page/Settings.cpp:658
> +    m_page = 0;

Should use nullptr.

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