Changeset 128921 in webkit

Timestamp:

Sep 18, 2012 12:26:13 PM (12 years ago)

Author:

yoli@rim.com

Message:

[BlackBerry] Popup page should reference the client with a weak pointer
​https://bugs.webkit.org/show_bug.cgi?id=97028

Reviewed by Rob Buis.

RIM PR# 209847.
Internally reviewed by Mike Fenton.

Store the pointer in a ref-coutned shared object, and clear the pointer
when the client is going to be destroyed, so it won't be accessed by
the JS function afterwards.

• WebCoreSupport/PagePopupBlackBerry.cpp:

(WebCore::PagePopupBlackBerry::PagePopupBlackBerry):
(WebCore::PagePopupBlackBerry::~PagePopupBlackBerry):
(WebCore::PagePopupBlackBerry::init):
(WebCore::setValueAndClosePopupCallback):
(WebCore::popUpExtensionFinalize):
(WebCore::PagePopupBlackBerry::installDOMFunction):
(WebCore::PagePopupBlackBerry::closePopup):

• WebCoreSupport/PagePopupBlackBerry.h:

(PagePopupBlackBerry):
(SharedClientPointer):
(WebCore::PagePopupBlackBerry::SharedClientPointer::SharedClientPointer):
(WebCore::PagePopupBlackBerry::SharedClientPointer::clear):
(WebCore::PagePopupBlackBerry::SharedClientPointer::get):

Location:

trunk/Source/WebKit/blackberry

Files:

• ChangeLog (modified) (1 diff)
• WebCoreSupport/PagePopupBlackBerry.cpp (modified) (8 diffs)
• WebCoreSupport/PagePopupBlackBerry.h (modified) (2 diffs)

trunk/Source/WebKit/blackberry/ChangeLog

@@ +1 @@
+2012-09-18  Yong Li  <yoli@rim.com>
+
+        [BlackBerry] Popup page should reference the client with a weak pointer
+        https://bugs.webkit.org/show_bug.cgi?id=97028
+
+        Reviewed by Rob Buis.
+
+        RIM PR# 209847.
+        Internally reviewed by Mike Fenton.
+
+        Store the pointer in a ref-coutned shared object, and clear the pointer 
+        when the client is going to be destroyed, so it won't be accessed by
+        the JS function afterwards.
+
+        * WebCoreSupport/PagePopupBlackBerry.cpp:
+        (WebCore::PagePopupBlackBerry::PagePopupBlackBerry):
+        (WebCore::PagePopupBlackBerry::~PagePopupBlackBerry):
+        (WebCore::PagePopupBlackBerry::init):
+        (WebCore::setValueAndClosePopupCallback):
+        (WebCore::popUpExtensionFinalize):
+        (WebCore::PagePopupBlackBerry::installDOMFunction):
+        (WebCore::PagePopupBlackBerry::closePopup):
+        * WebCoreSupport/PagePopupBlackBerry.h:
+        (PagePopupBlackBerry):
+        (SharedClientPointer):
+        (WebCore::PagePopupBlackBerry::SharedClientPointer::SharedClientPointer):
+        (WebCore::PagePopupBlackBerry::SharedClientPointer::clear):
+        (WebCore::PagePopupBlackBerry::SharedClientPointer::get):
+
 2012-09-18  Arvid Nilsson  <anilsson@rim.com>
 

trunk/Source/WebKit/blackberry/WebCoreSupport/PagePopupBlackBerry.cpp

@@ -51 +51 @@
     : m_webPagePrivate(webPage)
     , m_client(adoptPtr(client))
+    , m_sharedClientPointer(adoptRef(new PagePopupBlackBerry::SharedClientPointer(client)))
 {
     m_rect = IntRect(rect.x(), rect.y() - URL_BAR_HEIGHT, client->contentSize().width(), client->contentSize().height());
@@ -57 +58 @@
 PagePopupBlackBerry::~PagePopupBlackBerry()
 {
+    ASSERT(!m_sharedClientPointer->get());
 }
 
@@ -68 +70 @@
     generateHTML(webpage);
 
-    installDomFunction(webpage->d->mainFrame());
+    installDOMFunction(webpage->d->mainFrame());
 
     return true;
@@ -108 +110 @@
     JSObjectRef popUpObject = JSValueToObject(context,
             arguments[argumentCount - 1], 0);
-    PagePopupClient* client =
-            reinterpret_cast<PagePopupClient*>(JSObjectGetPrivate(popUpObject));
+    PagePopupBlackBerry::SharedClientPointer* client = reinterpret_cast<PagePopupBlackBerry::SharedClientPointer*>(JSObjectGetPrivate(popUpObject));
 
-    ASSERT(client);
-    client->setValueAndClosePopup(0, strArgs.data());
+    // Check the weak pointer as the owner page may have destroyed the popup.
+    if (client->get())
+        client->get()->setValueAndClosePopup(0, strArgs.data());
 
     return jsRetVal;
@@ -125 +127 @@
 static void popUpExtensionFinalize(JSObjectRef object)
 {
-    UNUSED_PARAM(object);
+    // Clear the reference. See installDOMFunction().
+    PagePopupBlackBerry::SharedClientPointer* client = reinterpret_cast<PagePopupBlackBerry::SharedClientPointer*>(JSObjectGetPrivate(object));
+    client->deref();
 }
 
@@ -139 +143 @@
 };
 
-void PagePopupBlackBerry::installDomFunction(Frame* frame)
+void PagePopupBlackBerry::installDOMFunction(Frame* frame)
 {
     JSDOMWindow* window = toJSDOMWindow(frame, mainThreadNormalWorld());
@@ -166 +170 @@
 
     JSObjectRef clientClassObject = JSObjectMake(context, clientClass, 0);
-    JSObjectSetPrivate(clientClassObject, reinterpret_cast<void*>(m_client.get()));
+
+    // Add a reference. See popUpExtensionFinalize.
+    m_sharedClientPointer->ref();
+    JSObjectSetPrivate(clientClassObject, m_sharedClientPointer.get());
 
     String name("popUp");
@@ -179 +186 @@
 void PagePopupBlackBerry::closePopup()
 {
+    // Prevent the popup page from accessing the client.
+    m_sharedClientPointer->clear();
+
     m_client->didClosePopup();
     m_webPagePrivate->client()->closePopupWebView();

trunk/Source/WebKit/blackberry/WebCoreSupport/PagePopupBlackBerry.h

@@ -22 +22 @@
 #include "IntRect.h"
 #include "PagePopup.h"
+#include <wtf/RefCounted.h>
+#include <wtf/RefPtr.h>
 
 
@@ -45 +47 @@
     bool init(BlackBerry::WebKit::WebPage*);
     void closePopup();
-    void installDomFunction(Frame*);
     void setRect();
-    void generateHTML(BlackBerry::WebKit::WebPage*);
+
+    class SharedClientPointer : public RefCounted<SharedClientPointer> {
+    public:
+        explicit SharedClientPointer(PagePopupClient* client) : m_client(client) { }
+        void clear() { m_client = 0; }
+        PagePopupClient* get() const { return m_client; }
+    private:
+        PagePopupClient* m_client;
+    };
 
 private:
+    void generateHTML(BlackBerry::WebKit::WebPage*);
+    void installDOMFunction(Frame*);
+
     BlackBerry::WebKit::WebPagePrivate* m_webPagePrivate;
     OwnPtr<PagePopupClient> m_client;
+    RefPtr<SharedClientPointer> m_sharedClientPointer;
     IntRect m_rect;
 };