Bug 90817

Summary:

[BlackBerry] PagePopupBlackBerry::closePopup() should always clear the pointer in WebPagePrivate

Product:

WebKit

Reporter:

Yong Li <yong.li.webkit>

Component:

WebKit BlackBerry

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

haizhang, mifenton, tonikitoo, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
the patch	none
again	none

Yong Li

Reported 2012-07-09 13:34:15 PDT

PagePopupBlackBerry::closePopup() should always clear the pointer in WebPagePrivate to avoid crashes.

Attachments
the patch (4.99 KB, patch) 2012-07-09 14:13 PDT, Yong Li	no flags	Details Formatted Diff Diff
again (4.96 KB, patch) 2012-07-09 14:37 PDT, Yong Li	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Yong Li

Comment 1 2012-07-09 14:13:59 PDT

Created attachment 151316 [details] the patch

George Staikos

Comment 2 2012-07-09 14:15:52 PDT

Comment on attachment 151316 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=151316&action=review > Source/WebKit/blackberry/WebCoreSupport/SelectPopupClient.cpp:147 > + ASSERT(m_element); Which one is it? Can it, or can it not be null?

Yong Li

Comment 3 2012-07-09 14:25:19 PDT

(In reply to comment #2) > (From update of attachment 151316 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=151316&action=review > > > Source/WebKit/blackberry/WebCoreSupport/SelectPopupClient.cpp:147 > > + ASSERT(m_element); > > Which one is it? Can it, or can it not be null? It is assigned through ctor, but cleared when the popup is closed. However the JS object is still there. It is theoretically possible the method is called by JS, e.g., window.popPup.setValueAndClosePopup(). Crystal, is that possible?

Crystal Zhang

Comment 4 2012-07-09 14:31:25 PDT

(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 151316 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=151316&action=review > > > > > Source/WebKit/blackberry/WebCoreSupport/SelectPopupClient.cpp:147 > > > + ASSERT(m_element); > > > > Which one is it? Can it, or can it not be null? > > It is assigned through ctor, but cleared when the popup is closed. However the JS object is still there. It is theoretically possible the method is called by JS, e.g., window.popPup.setValueAndClosePopup(). > > Crystal, is that possible? window.popPup.setValueAndClosePopup() will only be called by popup's JS file, if you want to be safe, we can add guard here, however ASSERT should be removed then, as they are conflicting each other.

Yong Li

Comment 5 2012-07-09 14:37:43 PDT

Created attachment 151322 [details] again

Rob Buis

Comment 6 2012-07-09 14:43:58 PDT

Comment on attachment 151322 [details] again Looks good.

WebKit Review Bot

Comment 7 2012-07-09 16:05:37 PDT

Comment on attachment 151322 [details] again Clearing flags on attachment: 151322 Committed r122162: <http://trac.webkit.org/changeset/122162>

WebKit Review Bot

Comment 8 2012-07-09 16:05:41 PDT

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.