2011-04-11 Anders Carlsson <andersca@apple.com>
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Apr 2011 21:26:28 +0000 (21:26 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Apr 2011 21:26:28 +0000 (21:26 +0000)
        Reviewed by Sam Weinig.

        First step towards simplifying WebPageProxy/WebProcessProxy/WebContext ownership
        https://bugs.webkit.org/show_bug.cgi?id=58266

        With this patch, the WKView holds a strong reference to a WebPageProxy. The
        WebPageProxy in turn holds a strong reference to its WebProcessProxy. Finally,
        The WebProcessProxy holds a strong reference to its WebContext.

        The WebContext holds a strong reference to the running WebProcessProxy which results
        in a reference cycle that's broken when the web process exits.

        The reason for is to avoid crashes where WebPageProxy::process() returns null if the web process
        has crashed but has not yet been relaunched.

        * UIProcess/WebContext.cpp:
        (WebKit::WebContext::disconnectProcess):
        Add comment.

        (WebKit::WebContext::createWebPage):
        Return a PassRefPtr.

        (WebKit::WebContext::relaunchProcessIfNecessary):
        Change this to return a WebPageProxy.

        * UIProcess/WebPageProxy.cpp:
        (WebKit::WebPageProxy::create):
        This now takes a PassRefPtr<WebProcessProxy>.

        (WebKit::WebPageProxy::WebPageProxy):
        Ditto.

        (WebKit::WebPageProxy::~WebPageProxy):
        Call close() if necessary.

        (WebKit::WebPageProxy::reattachToWebProcess):
        Replace the current process with the new process.

        * UIProcess/WebProcessProxy.cpp:
        (WebKit::WebProcessProxy::create):
        Take a PassRefPtr<WebContext>.

        (WebKit::WebProcessProxy::WebProcessProxy):
        Ditto.

        (WebKit::WebProcessProxy::webPage):
        Remove .get() now that the page map uses weak references.

        (WebKit::WebProcessProxy::createWebPage):
        This now returns the created web page proxy.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@83499 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/WebContext.cpp
Source/WebKit2/UIProcess/WebContext.h
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/WebProcessProxy.cpp
Source/WebKit2/UIProcess/WebProcessProxy.h

index f4c1730..1585e1b 100644 (file)
@@ -1,3 +1,56 @@
+2011-04-11  Anders Carlsson  <andersca@apple.com>
+
+        Reviewed by Sam Weinig.
+
+        First step towards simplifying WebPageProxy/WebProcessProxy/WebContext ownership
+        https://bugs.webkit.org/show_bug.cgi?id=58266
+
+        With this patch, the WKView holds a strong reference to a WebPageProxy. The 
+        WebPageProxy in turn holds a strong reference to its WebProcessProxy. Finally,
+        The WebProcessProxy holds a strong reference to its WebContext.
+
+        The WebContext holds a strong reference to the running WebProcessProxy which results
+        in a reference cycle that's broken when the web process exits.
+
+        The reason for is to avoid crashes where WebPageProxy::process() returns null if the web process
+        has crashed but has not yet been relaunched.
+
+        * UIProcess/WebContext.cpp:
+        (WebKit::WebContext::disconnectProcess):
+        Add comment.
+
+        (WebKit::WebContext::createWebPage):
+        Return a PassRefPtr.
+
+        (WebKit::WebContext::relaunchProcessIfNecessary):
+        Change this to return a WebPageProxy.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::create):
+        This now takes a PassRefPtr<WebProcessProxy>.
+
+        (WebKit::WebPageProxy::WebPageProxy):
+        Ditto.
+
+        (WebKit::WebPageProxy::~WebPageProxy):
+        Call close() if necessary.
+
+        (WebKit::WebPageProxy::reattachToWebProcess):
+        Replace the current process with the new process.
+
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::create):
+        Take a PassRefPtr<WebContext>.
+        
+        (WebKit::WebProcessProxy::WebProcessProxy):
+        Ditto.
+
+        (WebKit::WebProcessProxy::webPage):
+        Remove .get() now that the page map uses weak references.
+
+        (WebKit::WebProcessProxy::createWebPage):
+        This now returns the created web page proxy.
+
 2011-04-11  Adam Roben  <aroben@apple.com>
 
         Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply
index ff211d1..3a72787 100644 (file)
@@ -335,10 +335,11 @@ void WebContext::disconnectProcess(WebProcessProxy* process)
     m_pluginSiteDataManager->invalidate();
 #endif
 
+    // This can cause the web context to be destroyed.
     m_process = 0;
 }
 
-WebPageProxy* WebContext::createWebPage(PageClient* pageClient, WebPageGroup* pageGroup)
+PassRefPtr<WebPageProxy> WebContext::createWebPage(PageClient* pageClient, WebPageGroup* pageGroup)
 {
     ensureWebProcess();
 
@@ -348,9 +349,12 @@ WebPageProxy* WebContext::createWebPage(PageClient* pageClient, WebPageGroup* pa
     return m_process->createWebPage(pageClient, this, pageGroup);
 }
 
-void WebContext::relaunchProcessIfNecessary()
+WebProcessProxy* WebContext::relaunchProcessIfNecessary()
 {
     ensureWebProcess();
+
+    ASSERT(m_process);
+    return m_process.get();
 }
 
 void WebContext::download(WebPageProxy* initiatingPage, const ResourceRequest& request)
index 3f3f480..f47d34a 100644 (file)
@@ -84,9 +84,9 @@ public:
     // Disconnect the process from the context.
     void disconnectProcess(WebProcessProxy*);
 
-    WebPageProxy* createWebPage(PageClient*, WebPageGroup*);
+    PassRefPtr<WebPageProxy> createWebPage(PageClient*, WebPageGroup*);
 
-    void relaunchProcessIfNecessary();
+    WebProcessProxy* relaunchProcessIfNecessary();
 
     const String& injectedBundlePath() const { return m_injectedBundlePath; }
 
index 28536f9..9bf666b 100644 (file)
@@ -100,13 +100,14 @@ WKPageDebugPaintFlags WebPageProxy::s_debugPaintFlags = 0;
 static WTF::RefCountedLeakCounter webPageProxyCounter("WebPageProxy");
 #endif
 
-PassRefPtr<WebPageProxy> WebPageProxy::create(PageClient* pageClient, WebContext* context, WebPageGroup* pageGroup, uint64_t pageID)
+PassRefPtr<WebPageProxy> WebPageProxy::create(PageClient* pageClient, PassRefPtr<WebProcessProxy> process, WebContext* context, WebPageGroup* pageGroup, uint64_t pageID)
 {
-    return adoptRef(new WebPageProxy(pageClient, context, pageGroup, pageID));
+    return adoptRef(new WebPageProxy(pageClient, process, context, pageGroup, pageID));
 }
 
-WebPageProxy::WebPageProxy(PageClient* pageClient, WebContext* context, WebPageGroup* pageGroup, uint64_t pageID)
+WebPageProxy::WebPageProxy(PageClient* pageClient, PassRefPtr<WebProcessProxy> process, WebContext* context, WebPageGroup* pageGroup, uint64_t pageID)
     : m_pageClient(pageClient)
+    , m_process(process)
     , m_context(context)
     , m_pageGroup(pageGroup)
     , m_mainFrame(0)
@@ -162,6 +163,9 @@ WebPageProxy::WebPageProxy(PageClient* pageClient, WebContext* context, WebPageG
 
 WebPageProxy::~WebPageProxy()
 {
+    if (!m_isClosed)
+        close();
+
     WebContext::statistics().wkPageCount--;
 
     if (m_hasSpellDocumentTag)
@@ -239,9 +243,11 @@ void WebPageProxy::initializeContextMenuClient(const WKPageContextMenuClient* cl
 
 void WebPageProxy::reattachToWebProcess()
 {
+    ASSERT(!isValid());
+
     m_isValid = true;
 
-    context()->relaunchProcessIfNecessary();
+    m_process = context()->relaunchProcessIfNecessary();
     process()->addExistingWebPage(this, m_pageID);
 
     initializeWebPage();
index 0d35b31..25c74be 100644 (file)
@@ -158,7 +158,7 @@ class WebPageProxy : public APIObject, public WebPopupMenuProxy::Client {
 public:
     static const Type APIType = TypePage;
 
-    static PassRefPtr<WebPageProxy> create(PageClient*, WebContext*, WebPageGroup*, uint64_t pageID);
+    static PassRefPtr<WebPageProxy> create(PageClient*, PassRefPtr<WebProcessProxy>, WebContext*, WebPageGroup*, uint64_t pageID);
     virtual ~WebPageProxy();
 
     uint64_t pageID() const { return m_pageID; }
@@ -495,7 +495,7 @@ public:
     void linkClicked(const String&, const WebMouseEvent&);
  
 private:
-    WebPageProxy(PageClient*, WebContext*, WebPageGroup*, uint64_t pageID);
+    WebPageProxy(PageClient*, PassRefPtr<WebProcessProxy>, WebContext*, WebPageGroup*, uint64_t pageID);
 
     virtual Type type() const { return APIType; }
 
@@ -712,6 +712,7 @@ private:
     WebPageContextMenuClient m_contextMenuClient;
 
     OwnPtr<DrawingAreaProxy> m_drawingArea;
+    RefPtr<WebProcessProxy> m_process;
     RefPtr<WebContext> m_context;
     RefPtr<WebPageGroup> m_pageGroup;
     RefPtr<WebFrameProxy> m_mainFrame;
index e09b02a..a66337e 100644 (file)
@@ -59,12 +59,12 @@ static uint64_t generatePageID()
     return uniquePageID++;
 }
 
-PassRefPtr<WebProcessProxy> WebProcessProxy::create(WebContext* context)
+PassRefPtr<WebProcessProxy> WebProcessProxy::create(PassRefPtr<WebContext> context)
 {
     return adoptRef(new WebProcessProxy(context));
 }
 
-WebProcessProxy::WebProcessProxy(WebContext* context)
+WebProcessProxy::WebProcessProxy(PassRefPtr<WebContext> context)
     : m_responsivenessTimer(this)
     , m_context(context)
 {
@@ -163,17 +163,17 @@ void WebProcessProxy::terminate()
 
 WebPageProxy* WebProcessProxy::webPage(uint64_t pageID) const
 {
-    return m_pageMap.get(pageID).get();
+    return m_pageMap.get(pageID);
 }
 
-WebPageProxy* WebProcessProxy::createWebPage(PageClient* pageClient, WebContext* context, WebPageGroup* pageGroup)
+PassRefPtr<WebPageProxy> WebProcessProxy::createWebPage(PageClient* pageClient, WebContext* context, WebPageGroup* pageGroup)
 {
     ASSERT(context->process() == this);
 
     unsigned pageID = generatePageID();
-    RefPtr<WebPageProxy> webPage = WebPageProxy::create(pageClient, context, pageGroup, pageID);
-    m_pageMap.set(pageID, webPage);
-    return webPage.get();
+    RefPtr<WebPageProxy> webPage = WebPageProxy::create(pageClient, this, context, pageGroup, pageID);
+    m_pageMap.set(pageID, webPage.get());
+    return webPage.release();
 }
 
 void WebProcessProxy::addExistingWebPage(WebPageProxy* webPage, uint64_t pageID)
index 472692d..b2ef5f4 100644 (file)
@@ -56,7 +56,7 @@ public:
     typedef HashMap<uint64_t, RefPtr<WebFrameProxy> > WebFrameProxyMap;
     typedef HashMap<uint64_t, RefPtr<WebBackForwardListItem> > WebBackForwardListItemMap;
 
-    static PassRefPtr<WebProcessProxy> create(WebContext*);
+    static PassRefPtr<WebProcessProxy> create(PassRefPtr<WebContext>);
     ~WebProcessProxy();
 
     void terminate();
@@ -71,12 +71,12 @@ public:
         return m_connection.get(); 
     }
 
-    WebContext* context() const { return m_context; }
+    WebContext* context() const { return m_context.get(); }
 
     PlatformProcessIdentifier processIdentifier() const { return m_processLauncher->processIdentifier(); }
 
     WebPageProxy* webPage(uint64_t pageID) const;
-    WebPageProxy* createWebPage(PageClient*, WebContext*, WebPageGroup*);
+    PassRefPtr<WebPageProxy> createWebPage(PageClient*, WebContext*, WebPageGroup*);
     void addExistingWebPage(WebPageProxy*, uint64_t pageID);
     void removeWebPage(uint64_t pageID);
 
@@ -102,7 +102,7 @@ public:
     template<typename E, typename T> bool deprecatedSend(E messageID, uint64_t destinationID, const T& arguments);
 
 private:
-    explicit WebProcessProxy(WebContext*);
+    explicit WebProcessProxy(PassRefPtr<WebContext>);
 
     // Initializes the process or thread launcher which will begin launching the process.
     void connect();
@@ -155,9 +155,9 @@ private:
     RefPtr<ProcessLauncher> m_processLauncher;
     RefPtr<ThreadLauncher> m_threadLauncher;
 
-    WebContext* m_context;
+    RefPtr<WebContext> m_context;
 
-    HashMap<uint64_t, RefPtr<WebPageProxy> > m_pageMap;
+    HashMap<uint64_t, WebPageProxy*> m_pageMap;
     WebFrameProxyMap m_frameMap;
     WebBackForwardListItemMap m_backForwardListItemMap;
 };