[webkit-changes] [152733] trunk/Source

Tue, 16 Jul 2013 11:43:46 -0700

Title: [152733] trunk/Source
Revision
152733
Author
[email protected]
Date
2013-07-16 11:44:45 -0700 (Tue, 16 Jul 2013)

Log Message

Source/WebCore: Protect against the LayerFlushController being deleted inside its flushLayers() callback
https://bugs.webkit.org/show_bug.cgi?id=118741

Reviewed by Tim Horton.

It's possible (especially on iOS) for the LayerFlushController to be destroyed
inside its callback, via -[WebView _close]. Protect against this by making
it refcounted, and holding a ref across the callback.

Due to the odd relationship in which LayerFlushController owns its LayerFlushScheduler
by value, we achieve this by allowing subclasses of LayerFlushScheduler
to override runLoopObserverCallback(). WebViewLayerFlushScheduler uses the
override to protect the owner of the LayerFlushScheduler, which is the
LayerFlushController, when the callback is firing.

* WebCore.exp.in:
* platform/graphics/ca/LayerFlushScheduler.h: Make runLoopObserverCallback() and the dtor virtual.

Source/WebKit/mac: Protect against the LayerFlushController being deleted inside its flushLayers() callback
https://bugs.webkit.org/show_bug.cgi?id=118741
<rdar://problem/14402651>

Reviewed by Tim Horton.

It's possible (especially on iOS) for the LayerFlushController to be destroyed
inside its callback, via -[WebView _close]. Protect against this by making
it refcounted, and holding a ref across the callback.

Due to the odd relationship in which LayerFlushController owns its LayerFlushScheduler
by value, we achieve this by allowing subclasses of LayerFlushScheduler
to override runLoopObserverCallback(). WebViewLayerFlushScheduler uses the
override to protect the owner of the LayerFlushScheduler, which is the
LayerFlushController, when the callback is firing.

* WebView/WebView.mm:
(-[WebView _close]):
* WebView/WebViewData.h:
(WebViewLayerFlushScheduler::~WebViewLayerFlushScheduler):
(LayerFlushController::create):
* WebView/WebViewData.mm:
(LayerFlushController::invalidate):
(WebViewLayerFlushScheduler::WebViewLayerFlushScheduler):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (152732 => 152733)


--- trunk/Source/WebCore/ChangeLog	2013-07-16 18:38:09 UTC (rev 152732)
+++ trunk/Source/WebCore/ChangeLog	2013-07-16 18:44:45 UTC (rev 152733)
@@ -1,3 +1,23 @@
+2013-07-16  Simon Fraser  <[email protected]>
+
+        Protect against the LayerFlushController being deleted inside its flushLayers() callback
+        https://bugs.webkit.org/show_bug.cgi?id=118741
+
+        Reviewed by Tim Horton.
+
+        It's possible (especially on iOS) for the LayerFlushController to be destroyed
+        inside its callback, via -[WebView _close]. Protect against this by making
+        it refcounted, and holding a ref across the callback.
+        
+        Due to the odd relationship in which LayerFlushController owns its LayerFlushScheduler
+        by value, we achieve this by allowing subclasses of LayerFlushScheduler
+        to override runLoopObserverCallback(). WebViewLayerFlushScheduler uses the
+        override to protect the owner of the LayerFlushScheduler, which is the
+        LayerFlushController, when the callback is firing.
+
+        * WebCore.exp.in:
+        * platform/graphics/ca/LayerFlushScheduler.h: Make runLoopObserverCallback() and the dtor virtual.
+
 2013-07-16  Christophe Dumez  <[email protected]>
 
         Get rid of multiple inheritance support from the bindings generators

Modified: trunk/Source/WebCore/WebCore.exp.in (152732 => 152733)


--- trunk/Source/WebCore/WebCore.exp.in	2013-07-16 18:38:09 UTC (rev 152732)
+++ trunk/Source/WebCore/WebCore.exp.in	2013-07-16 18:44:45 UTC (rev 152733)
@@ -642,8 +642,11 @@
 __ZN7WebCore19LayerFlushScheduler6resumeEv
 __ZN7WebCore19LayerFlushScheduler7suspendEv
 __ZN7WebCore19LayerFlushScheduler8scheduleEv
+__ZN7WebCore19LayerFlushSchedulerC2EPNS_25LayerFlushSchedulerClientE
+__ZN7WebCore19LayerFlushSchedulerD1Ev
+__ZN7WebCore19LayerFlushSchedulerD2Ev
+__ZN7WebCore19LayerFlushScheduler23runLoopObserverCallbackEv
 __ZN7WebCore19LayerFlushSchedulerC1EPNS_25LayerFlushSchedulerClientE
-__ZN7WebCore19LayerFlushSchedulerD1Ev
 __ZN7WebCore19ResourceRequestBase11setHTTPBodyEN3WTF10PassRefPtrINS_8FormDataEEE
 __ZN7WebCore19ResourceRequestBase13setHTTPMethodERKN3WTF6StringE
 __ZN7WebCore19ResourceRequestBase18setHTTPHeaderFieldEPKcRKN3WTF6StringE

Modified: trunk/Source/WebCore/platform/graphics/ca/LayerFlushScheduler.h (152732 => 152733)


--- trunk/Source/WebCore/platform/graphics/ca/LayerFlushScheduler.h	2013-07-16 18:38:09 UTC (rev 152732)
+++ trunk/Source/WebCore/platform/graphics/ca/LayerFlushScheduler.h	2013-07-16 18:44:45 UTC (rev 152733)
@@ -38,7 +38,7 @@
     WTF_MAKE_NONCOPYABLE(LayerFlushScheduler);
 public:
     LayerFlushScheduler(LayerFlushSchedulerClient*);
-    ~LayerFlushScheduler();
+    virtual ~LayerFlushScheduler();
 
     void schedule();
     void invalidate();
@@ -55,7 +55,9 @@
 #if PLATFORM(MAC)
     RetainPtr<CFRunLoopObserverRef> m_runLoopObserver;
     static void runLoopObserverCallback(CFRunLoopObserverRef, CFRunLoopActivity, void* context);
-    void runLoopObserverCallback();
+
+protected:
+    virtual void runLoopObserverCallback();
 #endif
 };

Modified: trunk/Source/WebKit/mac/ChangeLog (152732 => 152733)


--- trunk/Source/WebKit/mac/ChangeLog	2013-07-16 18:38:09 UTC (rev 152732)
+++ trunk/Source/WebKit/mac/ChangeLog	2013-07-16 18:44:45 UTC (rev 152733)
@@ -1,3 +1,30 @@
+2013-07-16  Simon Fraser  <[email protected]>
+
+        Protect against the LayerFlushController being deleted inside its flushLayers() callback
+        https://bugs.webkit.org/show_bug.cgi?id=118741
+        <rdar://problem/14402651>
+
+        Reviewed by Tim Horton.
+        
+        It's possible (especially on iOS) for the LayerFlushController to be destroyed
+        inside its callback, via -[WebView _close]. Protect against this by making
+        it refcounted, and holding a ref across the callback.
+        
+        Due to the odd relationship in which LayerFlushController owns its LayerFlushScheduler
+        by value, we achieve this by allowing subclasses of LayerFlushScheduler
+        to override runLoopObserverCallback(). WebViewLayerFlushScheduler uses the
+        override to protect the owner of the LayerFlushScheduler, which is the
+        LayerFlushController, when the callback is firing.
+
+        * WebView/WebView.mm:
+        (-[WebView _close]):
+        * WebView/WebViewData.h:
+        (WebViewLayerFlushScheduler::~WebViewLayerFlushScheduler):
+        (LayerFlushController::create):
+        * WebView/WebViewData.mm:
+        (LayerFlushController::invalidate):
+        (WebViewLayerFlushScheduler::WebViewLayerFlushScheduler):
+
 2013-07-16  Jessie Berlin  <[email protected]>
 
         Fix some NSDictionary misuse pointed out by the clang static analyzer

Modified: trunk/Source/WebKit/mac/WebView/WebView.mm (152732 => 152733)


--- trunk/Source/WebKit/mac/WebView/WebView.mm	2013-07-16 18:38:09 UTC (rev 152732)
+++ trunk/Source/WebKit/mac/WebView/WebView.mm	2013-07-16 18:44:45 UTC (rev 152733)
@@ -1139,7 +1139,7 @@
 
 #if USE(ACCELERATED_COMPOSITING)
     if (_private->layerFlushController) {
-        _private->layerFlushController->invalidateObserver();
+        _private->layerFlushController->invalidate();
         _private->layerFlushController = nullptr;
     }
 #endif

Modified: trunk/Source/WebKit/mac/WebView/WebViewData.h (152732 => 152733)


--- trunk/Source/WebKit/mac/WebView/WebViewData.h	2013-07-16 18:38:09 UTC (rev 152732)
+++ trunk/Source/WebKit/mac/WebView/WebViewData.h	2013-07-16 18:44:45 UTC (rev 152733)
@@ -64,23 +64,40 @@
 extern int pluginDatabaseClientCount;
 
 #if USE(ACCELERATED_COMPOSITING)
-class LayerFlushController : public WebCore::LayerFlushSchedulerClient {
+class LayerFlushController;
+
+class WebViewLayerFlushScheduler : public WebCore::LayerFlushScheduler {
 public:
-    static PassOwnPtr<LayerFlushController> create(WebView* webView)
+    WebViewLayerFlushScheduler(LayerFlushController*);
+    virtual ~WebViewLayerFlushScheduler() { }
+
+private:
+    virtual void runLoopObserverCallback() OVERRIDE
     {
-        return adoptPtr(new LayerFlushController(webView));
+        RefPtr<LayerFlushController> protector = m_flushController;
+        WebCore::LayerFlushScheduler::runLoopObserverCallback();
     }
     
+    LayerFlushController* m_flushController;
+};
+
+class LayerFlushController : public RefCounted<LayerFlushController>, public WebCore::LayerFlushSchedulerClient {
+public:
+    static PassRefPtr<LayerFlushController> create(WebView* webView)
+    {
+        return adoptRef(new LayerFlushController(webView));
+    }
+    
     virtual bool flushLayers();
     
     void scheduleLayerFlush();
-    void invalidateObserver();
+    void invalidate();
     
 private:
     LayerFlushController(WebView*);
     
     WebView* m_webView;
-    WebCore::LayerFlushScheduler m_layerFlushScheduler;
+    WebViewLayerFlushScheduler m_layerFlushScheduler;
 };
 #endif
 
@@ -169,7 +186,7 @@
     // so that the NSView drawing is visually synchronized with CALayer updates.
     BOOL needsOneShotDrawingSynchronization;
     BOOL postsAcceleratedCompositingNotifications;
-    OwnPtr<LayerFlushController> layerFlushController;
+    RefPtr<LayerFlushController> layerFlushController;
 #endif
 
     NSPasteboard *insertionPasteboard;

Modified: trunk/Source/WebKit/mac/WebView/WebViewData.mm (152732 => 152733)


--- trunk/Source/WebKit/mac/WebView/WebViewData.mm	2013-07-16 18:38:09 UTC (rev 152732)
+++ trunk/Source/WebKit/mac/WebView/WebViewData.mm	2013-07-16 18:44:45 UTC (rev 152733)
@@ -48,9 +48,10 @@
     m_layerFlushScheduler.schedule();
 }
 
-void LayerFlushController::invalidateObserver()
+void LayerFlushController::invalidate()
 {
     m_layerFlushScheduler.invalidate();
+    m_webView = nullptr;
 }
 
 LayerFlushController::LayerFlushController(WebView* webView)
@@ -59,6 +60,12 @@
 {
     ASSERT_ARG(webView, webView);
 }
+
+WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(LayerFlushController* flushController)
+    : WebCore::LayerFlushScheduler(flushController)
+    , m_flushController(flushController)
+{
+}
 #endif
 
 @implementation WebViewPrivate

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to