Bug 75593

Summary:

null-check variable page before using it in FrameView::notifyPageThatContentAreaWillPaint.

Product:

WebKit

Reporter:

Yongjun Zhang <yongjun_zhang>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, webkit.review.bot, yongjun_zhang

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

Unspecified

Attachments:

Description	Flags
null-check variable 'page', to follow the usage pattern of m_frame->page() throughout the rest of FrameView.cpp.	none

Yongjun Zhang

Reported 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.

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 attachment proposed patch, testcase, etc.

Yongjun Zhang

Comment 1 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.

Yongjun Zhang

Comment 2 2012-01-04 18:06:43 PST

<rdar://problem/10603538>

Alexey Proskuryakov

Comment 3 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?

Yongjun Zhang

Comment 4 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.

Alexey Proskuryakov

Comment 5 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.

Yongjun Zhang

Comment 6 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.

WebKit Review Bot

Comment 7 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>

WebKit Review Bot

Comment 8 2012-01-05 18:13:49 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.