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

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Yong Li]

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

[Yong Li]

Created attachment 151316 [details]
the patch

[George Staikos]

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]

(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]

(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]

Created attachment 151322 [details]
again

[Rob Buis]

Comment on attachment 151322 [details]
again

Looks good.

[WebKit Review Bot]

Comment on attachment 151322 [details]
again

Clearing flags on attachment: 151322

Committed r122162: <http://trac.webkit.org/changeset/122162>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.