Bug 75593 - null-check variable page before using it in FrameView::notifyPageThatContentAreaWillPaint.
Summary: null-check variable page before using it in FrameView::notifyPageThatContentA...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-04 17:57 PST by Yongjun Zhang
Modified: 2012-01-05 18:13 PST (History)
3 users (show)

See Also:

Attachments
null-check variable 'page', to follow the usage pattern of m_frame->page() throughout the rest of FrameView.cpp. (1.26 KB, patch)
2012-01-04 18:03 PST, Yongjun Zhang 		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 Yongjun Zhang 2012-01-04 17:57:00 PST 
In FrameView::notifyPageThatContentAreaWillPaint(), we should null-check variable page, which is common usage pattern throughout the rest of FrameView.cpp.
Comment 1 Yongjun Zhang 2012-01-04 18:03:58 PST 
Created attachment 121199 [details]
null-check variable 'page', to follow the usage pattern of m_frame->page() throughout the rest of FrameView.cpp.
Comment 2 Yongjun Zhang 2012-01-04 18:06:43 PST 
<rdar://problem/10603538>
Comment 3 Alexey Proskuryakov 2012-01-05 10:30:27 PST 
Comment on attachment 121199 [details]
null-check variable 'page', to follow the usage pattern of m_frame->page() throughout the rest of FrameView.cpp.

There are several other places in FrameView.cpp that don't do the null check.

It would be strange if a pageless frame were to be painted! Does this fix an actual crash?
Comment 4 Yongjun Zhang 2012-01-05 14:36:12 PST 
(In reply to comment #3)
> (From update of attachment 121199 [details])
> There are several other places in FrameView.cpp that don't do the null check.
> 
> It would be strange if a pageless frame were to be painted! Does this fix an actual crash?

It fixed a hard-to-reproduce crash in iOS.
Comment 5 Alexey Proskuryakov 2012-01-05 16:56:01 PST 
Comment on attachment 121199 [details]
null-check variable 'page', to follow the usage pattern of m_frame->page() throughout the rest of FrameView.cpp.

I guess it's OK to add = another null check (there is enough confusion about when they are needed in this file already). 

It would be much better to find out why anyone would call notifyPageThatContentAreaWillPaint() when there is no page, and they are not painting, and fix that instead.
Comment 6 Yongjun Zhang 2012-01-05 17:13:57 PST 
(In reply to comment #5)
> (From update of attachment 121199 [details])
> I guess it's OK to add = another null check (there is enough confusion about when they are needed in this file already). 
> 
> It would be much better to find out why anyone would call notifyPageThatContentAreaWillPaint() when there is no page, and they are not painting, and fix that instead.

thanks Alexey!  Let's land this first and I will keep trying to find a reproducible case.
Comment 7 WebKit Review Bot 2012-01-05 18:13:44 PST 
Comment on attachment 121199 [details]
null-check variable 'page', to follow the usage pattern of m_frame->page() throughout the rest of FrameView.cpp.

Clearing flags on attachment: 121199

Committed r104260: <http://trac.webkit.org/changeset/104260>
Comment 8 WebKit Review Bot 2012-01-05 18:13:49 PST 
All reviewed patches have been landed.  Closing bug.