RESOLVED FIXED 90817
[BlackBerry] PagePopupBlackBerry::closePopup() should always clear the pointer in WebPagePrivate
https://bugs.webkit.org/show_bug.cgi?id=90817
Summary [BlackBerry] PagePopupBlackBerry::closePopup() should always clear the pointe...
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
again (4.96 KB, patch)
2012-07-09 14:37 PDT, Yong Li
no flags
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
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.