Bug 90817 - [BlackBerry] PagePopupBlackBerry::closePopup() should always clear the pointer in WebPagePrivate
Summary: [BlackBerry] PagePopupBlackBerry::closePopup() should always clear the pointe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-09 13:34 PDT by Yong Li
Modified: 2012-07-09 16:05 PDT (History)
4 users (show)

See Also:

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

Note You need to log in before you can comment on or make changes to this bug.
Description Yong Li 2012-07-09 13:34:15 PDT 
PagePopupBlackBerry::closePopup() should always clear the pointer in WebPagePrivate to avoid crashes.
Comment 1 Yong Li 2012-07-09 14:13:59 PDT 
Created attachment 151316 [details]
the patch
Comment 2 George Staikos 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?
Comment 3 Yong Li 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?
Comment 4 Crystal Zhang 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.
Comment 5 Yong Li 2012-07-09 14:37:43 PDT 
Created attachment 151322 [details]
again
Comment 6 Rob Buis 2012-07-09 14:43:58 PDT 
Comment on attachment 151322 [details]
again

Looks good.
Comment 7 WebKit Review Bot 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>
Comment 8 WebKit Review Bot 2012-07-09 16:05:41 PDT 
All reviewed patches have been landed.  Closing bug.