Store ViewSnapshots directly on the WebBackForwardListItem
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Jul 2014 20:33:05 +0000 (20:33 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Jul 2014 20:33:05 +0000 (20:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=134667
<rdar://problem/17082639>

Reviewed by Dan Bernstein.

Make ViewSnapshot a refcounted class. Store it directly on the back-forward item
instead of in a side map referenced by UUID. Switch to a very simple LRU eviction model for now.
This fixes a ton of snapshot management bugs; for example, we would start throwing out snapshots
in the page that was actively being interacted with *first* when evicting snapshots, instead of
preferring older snapshots. Additionally, we would not throw away snapshots when back forward items
became unreachable.

There is definitely room for improvement of the eviction mechanism, but this is closer to a time-tested implementation.

* Shared/SessionState.h:
Keep a ViewSnapshot instead of a UUID on the BackForwardListItemState.

* Shared/WebBackForwardListItem.h:
Fix some indented namespace contents.

(WebKit::WebBackForwardListItem::snapshot):
(WebKit::WebBackForwardListItem::setSnapshot):
(WebKit::WebBackForwardListItem::setSnapshotUUID): Deleted.
(WebKit::WebBackForwardListItem::snapshotUUID): Deleted.
Switch the snapshot getter/setter to operate on ViewSnapshots instead of UUIDs.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _takeViewSnapshot]):
* UIProcess/API/Cocoa/WKWebViewInternal.h:
* UIProcess/API/mac/WKView.mm:
(-[WKView _takeViewSnapshot]):
* UIProcess/API/mac/WKViewInternal.h:
* UIProcess/PageClient.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::takeViewSnapshot):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::takeViewSnapshot):
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::takeViewSnapshot):
Adopt ViewSnapshot::create, return a PassRefPtr, and class-ify ViewSnapshot.

* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::beginSwipeGesture):
(WebKit::ViewGestureController::endSwipeGesture):
* UIProcess/mac/ViewGestureController.h:
* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::shouldUseSnapshotForSize):
(WebKit::ViewGestureController::beginSwipeGesture):
(WebKit::ViewGestureController::endSwipeGesture):
Grab the ViewSnapshot directly from the WebBackForwardListItem, and adopt the new functions.

* UIProcess/ios/WebMemoryPressureHandlerIOS.mm:
(WebKit::WebMemoryPressureHandler::WebMemoryPressureHandler):
Rename discardSnapshots to discardSnapshotImages, because we're really only discarding
the images; the render tree size/background color "snapshot" remains and is useful.

* UIProcess/mac/ViewSnapshotStore.h:
(WebKit::ViewSnapshot::setRenderTreeSize):
(WebKit::ViewSnapshot::renderTreeSize):
(WebKit::ViewSnapshot::setBackgroundColor):
(WebKit::ViewSnapshot::backgroundColor):
(WebKit::ViewSnapshot::setDeviceScaleFactor):
(WebKit::ViewSnapshot::deviceScaleFactor):
(WebKit::ViewSnapshot::imageSizeInBytes):
(WebKit::ViewSnapshot::surface):
(WebKit::ViewSnapshot::size):
(WebKit::ViewSnapshot::creationTime):
Make ViewSnapshot a refcounted class.
Add create functions which take an image (or slot ID), and relevant sizes.
It is expected that a ViewSnapshot is created with an image, and it is only possible
to remove that image, never to replace it. A new ViewSnapshot is required in that case.
Add setters for things that ViewSnapshotStore sets on the snapshot after the PageClient
retrieves it from the view. Add getters for things that the ViewGestureControllers need.

Remove removeSnapshotImage, getSnapshot, and the snapshot map.

* UIProcess/mac/ViewSnapshotStore.mm:
(WebKit::ViewSnapshotStore::~ViewSnapshotStore):
(WebKit::ViewSnapshotStore::didAddImageToSnapshot):
(WebKit::ViewSnapshotStore::willRemoveImageFromSnapshot):
Manage m_snapshotCacheSize and m_snapshotsWithImages via didAddImageToSnapshot and willRemoveImageFromSnapshot.
willRemoveImageFromSnapshot will -always- be called before the ViewSnapshot is destroyed.

(WebKit::ViewSnapshotStore::pruneSnapshots):
Switch to a simple LRU eviction model. As previously mentioned, it's possible to do better, but
this is much less broken than the previous implementation.

(WebKit::ViewSnapshotStore::recordSnapshot):
(WebKit::ViewSnapshotStore::discardSnapshotImages):
(WebKit::ViewSnapshot::create):
(WebKit::ViewSnapshot::ViewSnapshot):
(WebKit::ViewSnapshot::~ViewSnapshot):
(WebKit::ViewSnapshot::hasImage):
(WebKit::ViewSnapshot::clearImage):
(WebKit::ViewSnapshot::asLayerContents):
If a surface is Empty when it comes back from being volatile, throw away the surface
and notify the Store to remove it from m_snapshotCacheSize (via clearImage()).

(WebKit::ViewSnapshotStore::removeSnapshotImage): Deleted.
(WebKit::ViewSnapshotStore::getSnapshot): Deleted.
(WebKit::ViewSnapshotStore::discardSnapshots): Deleted.

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

20 files changed:
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/SessionState.h
Source/WebKit2/Shared/WebBackForwardListItem.h
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/API/Cocoa/WKWebViewInternal.h
Source/WebKit2/UIProcess/API/mac/WKView.mm
Source/WebKit2/UIProcess/API/mac/WKViewInternal.h
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm
Source/WebKit2/UIProcess/ios/WebMemoryPressureHandlerIOS.mm
Source/WebKit2/UIProcess/mac/PageClientImpl.h
Source/WebKit2/UIProcess/mac/PageClientImpl.mm
Source/WebKit2/UIProcess/mac/ViewGestureController.h
Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm
Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h
Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm

index 5565eef4ea8e086bdc46aec5ffa9f22301744065..909234d7d0147ead330355ce49e3eb1607129731 100644 (file)
@@ -1,3 +1,111 @@
+2014-07-10  Tim Horton  <timothy_horton@apple.com>
+
+        Store ViewSnapshots directly on the WebBackForwardListItem
+        https://bugs.webkit.org/show_bug.cgi?id=134667
+        <rdar://problem/17082639>
+
+        Reviewed by Dan Bernstein.
+
+        Make ViewSnapshot a refcounted class. Store it directly on the back-forward item
+        instead of in a side map referenced by UUID. Switch to a very simple LRU eviction model for now.
+        This fixes a ton of snapshot management bugs; for example, we would start throwing out snapshots
+        in the page that was actively being interacted with *first* when evicting snapshots, instead of
+        preferring older snapshots. Additionally, we would not throw away snapshots when back forward items
+        became unreachable.
+
+        There is definitely room for improvement of the eviction mechanism, but this is closer to a time-tested implementation.
+
+        * Shared/SessionState.h:
+        Keep a ViewSnapshot instead of a UUID on the BackForwardListItemState.
+
+        * Shared/WebBackForwardListItem.h:
+        Fix some indented namespace contents.
+
+        (WebKit::WebBackForwardListItem::snapshot):
+        (WebKit::WebBackForwardListItem::setSnapshot):
+        (WebKit::WebBackForwardListItem::setSnapshotUUID): Deleted.
+        (WebKit::WebBackForwardListItem::snapshotUUID): Deleted.
+        Switch the snapshot getter/setter to operate on ViewSnapshots instead of UUIDs.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _takeViewSnapshot]):
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        * UIProcess/API/mac/WKView.mm:
+        (-[WKView _takeViewSnapshot]):
+        * UIProcess/API/mac/WKViewInternal.h:
+        * UIProcess/PageClient.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::takeViewSnapshot):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::takeViewSnapshot):
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::takeViewSnapshot):
+        Adopt ViewSnapshot::create, return a PassRefPtr, and class-ify ViewSnapshot.
+
+        * UIProcess/ios/ViewGestureControllerIOS.mm:
+        (WebKit::ViewGestureController::beginSwipeGesture):
+        (WebKit::ViewGestureController::endSwipeGesture):
+        * UIProcess/mac/ViewGestureController.h:
+        * UIProcess/mac/ViewGestureControllerMac.mm:
+        (WebKit::ViewGestureController::shouldUseSnapshotForSize):
+        (WebKit::ViewGestureController::beginSwipeGesture):
+        (WebKit::ViewGestureController::endSwipeGesture):
+        Grab the ViewSnapshot directly from the WebBackForwardListItem, and adopt the new functions.
+
+        * UIProcess/ios/WebMemoryPressureHandlerIOS.mm:
+        (WebKit::WebMemoryPressureHandler::WebMemoryPressureHandler):
+        Rename discardSnapshots to discardSnapshotImages, because we're really only discarding
+        the images; the render tree size/background color "snapshot" remains and is useful.
+
+        * UIProcess/mac/ViewSnapshotStore.h:
+        (WebKit::ViewSnapshot::setRenderTreeSize):
+        (WebKit::ViewSnapshot::renderTreeSize):
+        (WebKit::ViewSnapshot::setBackgroundColor):
+        (WebKit::ViewSnapshot::backgroundColor):
+        (WebKit::ViewSnapshot::setDeviceScaleFactor):
+        (WebKit::ViewSnapshot::deviceScaleFactor):
+        (WebKit::ViewSnapshot::imageSizeInBytes):
+        (WebKit::ViewSnapshot::surface):
+        (WebKit::ViewSnapshot::size):
+        (WebKit::ViewSnapshot::creationTime):
+        Make ViewSnapshot a refcounted class.
+        Add create functions which take an image (or slot ID), and relevant sizes.
+        It is expected that a ViewSnapshot is created with an image, and it is only possible
+        to remove that image, never to replace it. A new ViewSnapshot is required in that case.
+        Add setters for things that ViewSnapshotStore sets on the snapshot after the PageClient
+        retrieves it from the view. Add getters for things that the ViewGestureControllers need.
+
+        Remove removeSnapshotImage, getSnapshot, and the snapshot map.
+
+        * UIProcess/mac/ViewSnapshotStore.mm:
+        (WebKit::ViewSnapshotStore::~ViewSnapshotStore):
+        (WebKit::ViewSnapshotStore::didAddImageToSnapshot):
+        (WebKit::ViewSnapshotStore::willRemoveImageFromSnapshot):
+        Manage m_snapshotCacheSize and m_snapshotsWithImages via didAddImageToSnapshot and willRemoveImageFromSnapshot.
+        willRemoveImageFromSnapshot will -always- be called before the ViewSnapshot is destroyed.
+
+        (WebKit::ViewSnapshotStore::pruneSnapshots):
+        Switch to a simple LRU eviction model. As previously mentioned, it's possible to do better, but
+        this is much less broken than the previous implementation.
+
+        (WebKit::ViewSnapshotStore::recordSnapshot):
+        (WebKit::ViewSnapshotStore::discardSnapshotImages):
+        (WebKit::ViewSnapshot::create):
+        (WebKit::ViewSnapshot::ViewSnapshot):
+        (WebKit::ViewSnapshot::~ViewSnapshot):
+        (WebKit::ViewSnapshot::hasImage):
+        (WebKit::ViewSnapshot::clearImage):
+        (WebKit::ViewSnapshot::asLayerContents):
+        If a surface is Empty when it comes back from being volatile, throw away the surface
+        and notify the Store to remove it from m_snapshotCacheSize (via clearImage()).
+
+        (WebKit::ViewSnapshotStore::removeSnapshotImage): Deleted.
+        (WebKit::ViewSnapshotStore::getSnapshot): Deleted.
+        (WebKit::ViewSnapshotStore::discardSnapshots): Deleted.
+
 2014-07-10  Beth Dakin  <bdakin@apple.com>
 
         Need Setting/WKPreference that allows clients to prevent scrollbars from drawing 
index 76e41c263cc9f4c09a003f14c0dbdccb0f1dd2d2..8d19c66367c7faa98f43e08cde9f9765c142261f 100644 (file)
@@ -26,6 +26,7 @@
 #ifndef SessionState_h
 #define SessionState_h
 
+#include "ViewSnapshotStore.h"
 #include <WebCore/FloatRect.h>
 #include <WebCore/IntRect.h>
 #include <WebCore/URL.h>
@@ -120,8 +121,7 @@ struct BackForwardListItemState {
     uint64_t identifier;
 
     PageState pageState;
-    // FIXME: This should hold the snapshot itself, not its UUID.
-    String snapshotUUID;
+    RefPtr<ViewSnapshot> snapshot;
 };
 
 struct BackForwardListState {
index 0960c79ce8cc817daef70ef67f60856c7b7fbecb..feac19c9715519a1ce16ba722613f58b2a980668 100644 (file)
@@ -36,8 +36,8 @@ class Data;
 }
 
 namespace IPC {
-    class ArgumentDecoder;
-    class ArgumentEncoder;
+class ArgumentDecoder;
+class ArgumentEncoder;
 }
 
 namespace WebKit {
@@ -56,9 +56,9 @@ public:
     const String& originalURL() const { return m_itemState.pageState.mainFrameState.originalURLString; }
     const String& url() const { return m_itemState.pageState.mainFrameState.urlString; }
     const String& title() const { return m_itemState.pageState.title; }
-    
-    void setSnapshotUUID(const String& uuid) { m_itemState.snapshotUUID = uuid; }
-    const String& snapshotUUID() const { return m_itemState.snapshotUUID; }
+
+    ViewSnapshot* snapshot() const { return m_itemState.snapshot.get(); }
+    void setSnapshot(PassRefPtr<ViewSnapshot> snapshot) { m_itemState.snapshot = snapshot; }
 
     static uint64_t highedUsedItemID();
 
index 37cfe7b6cab00a28bd8fa5fbc03b429fc9b19fd4..85c49e966208148937991ba5288a6bbc768e5ec3 100644 (file)
@@ -899,24 +899,23 @@ static void changeContentOffsetBoundedInValidRange(UIScrollView *scrollView, Web
     _scaleToRestore = scale;
 }
 
-- (WebKit::ViewSnapshot)_takeViewSnapshot
+- (PassRefPtr<WebKit::ViewSnapshot>)_takeViewSnapshot
 {
     float deviceScale = WKGetScreenScaleFactor();
     CGSize snapshotSize = self.bounds.size;
     snapshotSize.width *= deviceScale;
     snapshotSize.height *= deviceScale;
 
-    WebKit::ViewSnapshot snapshot;
-    snapshot.slotID = [WebKit::ViewSnapshotStore::snapshottingContext() createImageSlot:snapshotSize hasAlpha:YES];
+    uint32_t slotID = [WebKit::ViewSnapshotStore::snapshottingContext() createImageSlot:snapshotSize hasAlpha:YES];
 
-    CATransform3D transform = CATransform3DMakeScale(deviceScale, deviceScale, 1);
-    CARenderServerCaptureLayerWithTransform(MACH_PORT_NULL, self.layer.context.contextId, (uint64_t)self.layer, snapshot.slotID, 0, 0, &transform);
+    if (!slotID)
+        return nullptr;
 
-    snapshot.size = WebCore::expandedIntSize(WebCore::FloatSize(snapshotSize));
-    snapshot.imageSizeInBytes = snapshotSize.width * snapshotSize.height * 4;
-    snapshot.backgroundColor = _page->pageExtendedBackgroundColor();
+    CATransform3D transform = CATransform3DMakeScale(deviceScale, deviceScale, 1);
+    CARenderServerCaptureLayerWithTransform(MACH_PORT_NULL, self.layer.context.contextId, (uint64_t)self.layer, slotID, 0, 0, &transform);
 
-    return snapshot;
+    WebCore::IntSize imageSize = WebCore::expandedIntSize(WebCore::FloatSize(snapshotSize));
+    return WebKit::ViewSnapshot::create(slotID, imageSize, imageSize.width() * imageSize.height() * 4);
 }
 
 - (void)_zoomToPoint:(WebCore::FloatPoint)point atScale:(double)scale
index cf90a153acf3b861363206d904fe87f34d40d294..4aa97976143c57a318fff7e4bd0163b9ac62ca0a 100644 (file)
@@ -50,9 +50,9 @@ struct Highlight;
 }
 
 namespace WebKit {
+class ViewSnapshot;
 class WebPageProxy;
 struct PrintInfo;
-struct ViewSnapshot;
 }
 
 @class _WKFrameHandle;
@@ -78,7 +78,7 @@ struct ViewSnapshot;
 - (void)_restorePageStateToExposedRect:(WebCore::FloatRect)exposedRect scale:(double)scale;
 - (void)_restorePageStateToUnobscuredCenter:(WebCore::FloatPoint)center scale:(double)scale;
 
-- (WebKit::ViewSnapshot)_takeViewSnapshot;
+- (PassRefPtr<WebKit::ViewSnapshot>)_takeViewSnapshot;
 
 - (void)_scrollToContentOffset:(WebCore::FloatPoint)contentOffset;
 - (BOOL)_scrollToRect:(WebCore::FloatRect)targetRect origin:(WebCore::FloatPoint)origin minimumScrollDistance:(float)minimumScrollDistance;
index 11dd71135f16e7c2537521b446ff4d2123e8fabb..d895427dd29c00fdde060ded330e3e37f519ca97 100644 (file)
@@ -3101,15 +3101,13 @@ static void* keyValueObservingContext = &keyValueObservingContext;
     return _data->_rootLayer.get();
 }
 
-- (ViewSnapshot)_takeViewSnapshot
+- (PassRefPtr<ViewSnapshot>)_takeViewSnapshot
 {
     NSWindow *window = self.window;
 
-    ViewSnapshot snapshot;
-
     CGSWindowID windowID = (CGSWindowID)[window windowNumber];
     if (!windowID || ![window isVisible])
-        return snapshot;
+        return nullptr;
 
     RetainPtr<CGImageRef> windowSnapshotImage = adoptCF(CGWindowListCreateImage(CGRectNull, kCGWindowListOptionIncludingWindow, windowID, kCGWindowImageBoundsIgnoreFraming | kCGWindowImageShouldBeOpaque));
 
@@ -3141,15 +3139,12 @@ static void* keyValueObservingContext = &keyValueObservingContext;
 
     auto croppedSnapshotImage = adoptCF(CGImageCreateWithImageInRect(windowSnapshotImage.get(), NSRectToCGRect([window convertRectToBacking:croppedImageRect])));
 
-    snapshot.surface = IOSurface::createFromImage(croppedSnapshotImage.get());
-    snapshot.surface->setIsVolatile(true);
-
-    IntSize imageSize(CGImageGetWidth(croppedSnapshotImage.get()), CGImageGetHeight(croppedSnapshotImage.get()));
-    snapshot.size = imageSize;
-    snapshot.imageSizeInBytes = imageSize.width() * imageSize.height() * 4;
-    snapshot.backgroundColor = _data->_page->pageExtendedBackgroundColor();
+    auto surface = IOSurface::createFromImage(croppedSnapshotImage.get());
+    if (!surface)
+        return nullptr;
+    surface->setIsVolatile(true);
 
-    return snapshot;
+    return ViewSnapshot::create(surface.get(), surface->size(), surface->totalBytes());
 }
 
 - (void)_wheelEventWasNotHandledByWebCore:(NSEvent *)event
index 31f2541cbf83c3da2cb55ed94f5716b7e01174e7..7ffa9d773dc6098503694e473b6e4718a13727a3 100644 (file)
@@ -47,10 +47,10 @@ namespace WebKit {
 class DrawingAreaProxy;
 class FindIndicator;
 class LayerTreeContext;
+class ViewSnapshot;
 class WebContext;
 struct ColorSpaceData;
 struct EditorState;
-struct ViewSnapshot;
 struct WebPageConfiguration;
 }
 
@@ -83,7 +83,7 @@ struct WebPageConfiguration;
 - (void)_setAcceleratedCompositingModeRootLayer:(CALayer *)rootLayer;
 - (CALayer *)_acceleratedCompositingModeRootLayer;
 
-- (WebKit::ViewSnapshot)_takeViewSnapshot;
+- (PassRefPtr<WebKit::ViewSnapshot>)_takeViewSnapshot;
 - (void)_wheelEventWasNotHandledByWebCore:(NSEvent *)event;
 
 - (void)_setAccessibilityWebProcessToken:(NSData *)data;
index 7ca46e6b6af5dc483659fd8dff1a7b8eb64b8fe1..39eeb1306f42bb54741469d5d7bb0c958d0764fc 100644 (file)
@@ -57,10 +57,10 @@ class DrawingAreaProxy;
 class FindIndicator;
 class NativeWebKeyboardEvent;
 class RemoteLayerTreeTransaction;
+class ViewSnapshot;
 class WebContextMenuProxy;
 class WebEditCommandProxy;
 class WebPopupMenuProxy;
-struct ViewSnapshot;
 
 #if ENABLE(TOUCH_EVENTS)
 class NativeWebTouchEvent;
@@ -180,7 +180,7 @@ public:
     virtual void makeFirstResponder() = 0;
     virtual void setAcceleratedCompositingRootLayer(LayerOrView *) = 0;
     virtual LayerOrView *acceleratedCompositingRootLayer() const = 0;
-    virtual ViewSnapshot takeViewSnapshot() = 0;
+    virtual PassRefPtr<ViewSnapshot> takeViewSnapshot() = 0;
     virtual void wheelEventWasNotHandledByWebCore(const NativeWebWheelEvent&) = 0;
     virtual void clearCustomSwipeViews() = 0;
 #endif
index 1f15641c6c95f70651d0a1da7701d75030be9c28..3d98714bcf09fcf30da1519d9e3ba3d64454f219 100644 (file)
@@ -4944,7 +4944,7 @@ void WebPageProxy::dictationAlternatives(uint64_t dictationContext, Vector<Strin
 #endif // PLATFORM(MAC)
 
 #if PLATFORM(COCOA)
-ViewSnapshot WebPageProxy::takeViewSnapshot()
+PassRefPtr<ViewSnapshot> WebPageProxy::takeViewSnapshot()
 {
     return m_pageClient.takeViewSnapshot();
 }
index bd956c539b18fbcb6cfc1a69876a9eeebe404dcb..42177c1baf612d0e1547eaf6a8baf774986a037a 100644 (file)
@@ -152,6 +152,7 @@ class PageClient;
 class RemoteLayerTreeTransaction;
 class RemoteScrollingCoordinatorProxy;
 class StringPairVector;
+class ViewSnapshot;
 class VisitedLinkProvider;
 class WebBackForwardList;
 class WebBackForwardListItem;
@@ -173,7 +174,6 @@ struct EditingRange;
 struct EditorState;
 struct PlatformPopupMenuData;
 struct PrintInfo;
-struct ViewSnapshot;
 struct WebPopupItem;
 
 #if ENABLE(VIBRATION)
@@ -884,7 +884,7 @@ public:
     void recordNavigationSnapshot();
 
 #if PLATFORM(COCOA)
-    ViewSnapshot takeViewSnapshot();
+    PassRefPtr<ViewSnapshot> takeViewSnapshot();
 #endif
 
 #if ENABLE(SUBTLE_CRYPTO)
index 22ecdf85ee525ca9310ce24fcd1f1113dd2a50b9..3a3199684f2c1ba92e9d895e0dd2b35a5b6c1111 100644 (file)
@@ -105,7 +105,7 @@ private:
     virtual LayerOrView *acceleratedCompositingRootLayer() const override;
     virtual LayerHostingMode viewLayerHostingMode() override { return LayerHostingMode::OutOfProcess; }
 
-    virtual ViewSnapshot takeViewSnapshot() override;
+    virtual PassRefPtr<ViewSnapshot> takeViewSnapshot() override;
     virtual void wheelEventWasNotHandledByWebCore(const NativeWebWheelEvent&) override;
     virtual void clearCustomSwipeViews() override;
 
index 2a0cdf69be4e660e72a5776348735e0cf771db82..64fe0c87acd855986e8826eb9f4c1968fb817bc2 100644 (file)
@@ -470,7 +470,7 @@ LayerOrView *PageClientImpl::acceleratedCompositingRootLayer() const
     return nullptr;
 }
 
-ViewSnapshot PageClientImpl::takeViewSnapshot()
+PassRefPtr<ViewSnapshot> PageClientImpl::takeViewSnapshot()
 {
     return [m_webView _takeViewSnapshot];
 }
index 22429fd935649da9643f7656c94825cb5b3a331a..914a8470bc3a0224b882c8d7608af399d61c2e7f 100644 (file)
@@ -168,14 +168,13 @@ void ViewGestureController::beginSwipeGesture(_UINavigationInteractiveTransition
     m_snapshotView = adoptNS([[UIView alloc] initWithFrame:liveSwipeViewFrame]);
 
     RetainPtr<UIColor> backgroundColor = [UIColor whiteColor];
-    ViewSnapshot snapshot;
-    if (ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot)) {
+    if (ViewSnapshot* snapshot = targetItem->snapshot()) {
         float deviceScaleFactor = m_webPageProxy.deviceScaleFactor();
         FloatSize swipeLayerSizeInDeviceCoordinates(liveSwipeViewFrame.size);
         swipeLayerSizeInDeviceCoordinates.scale(deviceScaleFactor);
-        if (snapshot.hasImage() && snapshot.size == swipeLayerSizeInDeviceCoordinates && deviceScaleFactor == snapshot.deviceScaleFactor)
-            [m_snapshotView layer].contents = snapshot.asLayerContents();
-        Color coreColor = snapshot.backgroundColor;
+        if (snapshot->hasImage() && snapshot->size() == swipeLayerSizeInDeviceCoordinates && deviceScaleFactor == snapshot->deviceScaleFactor())
+            [m_snapshotView layer].contents = snapshot->asLayerContents();
+        Color coreColor = snapshot->backgroundColor();
         if (coreColor.isValid())
             backgroundColor = adoptNS([[UIColor alloc] initWithCGColor:cachedCGColor(coreColor, ColorSpaceDeviceRGB)]);
     }
@@ -244,10 +243,9 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
         return;
     }
 
-    ViewSnapshot snapshot;
     m_snapshotRemovalTargetRenderTreeSize = 0;
-    if (ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot))
-        m_snapshotRemovalTargetRenderTreeSize = snapshot.renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
+    if (ViewSnapshot* snapshot = targetItem->snapshot())
+        m_snapshotRemovalTargetRenderTreeSize = snapshot->renderTreeSize() * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
 
     // We don't want to replace the current back-forward item's snapshot
     // like we normally would when going back or forward, because we are
index 7d4999a9603352d27a6eba5930a5955b3fd338ff..523d0c8f1465e7cba8412d23b4602a99fa73a604 100644 (file)
@@ -48,7 +48,7 @@ WebMemoryPressureHandler::WebMemoryPressureHandler()
     dispatch_source_t source = dispatch_source_create(DISPATCH_SOURCE_TYPE_MEMORYSTATUS, 0, DISPATCH_MEMORYSTATUS_PRESSURE_WARN, dispatch_get_main_queue());
     dispatch_set_context(source, this);
     dispatch_source_set_event_handler(source, ^{
-        ViewSnapshotStore::shared().discardSnapshots();
+        ViewSnapshotStore::shared().discardSnapshotImages();
     });
     dispatch_resume(source);
 }
index 611fa978837fa670d22714375766a12e7d6763f7..f57c816b22c4a24df47e7143a05c0abb4dd8dd83 100644 (file)
@@ -124,7 +124,7 @@ private:
     virtual void exitAcceleratedCompositingMode();
     virtual void updateAcceleratedCompositingMode(const LayerTreeContext&);
 
-    virtual ViewSnapshot takeViewSnapshot() override;
+    virtual PassRefPtr<ViewSnapshot> takeViewSnapshot() override;
     virtual void wheelEventWasNotHandledByWebCore(const NativeWebWheelEvent&) override;
     virtual void clearCustomSwipeViews() override;
 
index 47a8746dcd5461b3bd4086ee857319fe93d7037e..e1b5a029a9a0176fcbf25dc004648d0d42441cc3 100644 (file)
@@ -507,7 +507,7 @@ CALayer *PageClientImpl::acceleratedCompositingRootLayer() const
     return m_wkView._acceleratedCompositingModeRootLayer;
 }
 
-ViewSnapshot PageClientImpl::takeViewSnapshot()
+PassRefPtr<ViewSnapshot> PageClientImpl::takeViewSnapshot()
 {
     return [m_wkView _takeViewSnapshot];
 }
index b6085a2241be86e4b5bc5228927e5cb5e8912306..8e12d12f0663e620f0ff2350d46bb70bf892ca59 100644 (file)
@@ -50,7 +50,7 @@ class IOSurface;
 
 namespace WebKit {
 
-struct ViewSnapshot;
+class ViewSnapshot;
 class WebBackForwardListItem;
 class WebPageProxy;
 
index 15492913727018544b216d73dcc6edd3bf57e9fd..b96a59da5dc243c79ebe022e50bc2fe246280067 100644 (file)
@@ -454,12 +454,12 @@ CALayer *ViewGestureController::determineLayerAdjacentToSnapshotForParent(SwipeD
 bool ViewGestureController::shouldUseSnapshotForSize(ViewSnapshot& snapshot, FloatSize swipeLayerSize, float topContentInset)
 {
     float deviceScaleFactor = m_webPageProxy.deviceScaleFactor();
-    if (snapshot.deviceScaleFactor != deviceScaleFactor)
+    if (snapshot.deviceScaleFactor() != deviceScaleFactor)
         return false;
 
     FloatSize unobscuredSwipeLayerSizeInDeviceCoordinates = swipeLayerSize - FloatSize(0, topContentInset);
     unobscuredSwipeLayerSizeInDeviceCoordinates.scale(deviceScaleFactor);
-    if (snapshot.size != unobscuredSwipeLayerSizeInDeviceCoordinates)
+    if (snapshot.size() != unobscuredSwipeLayerSizeInDeviceCoordinates)
         return false;
 
     return true;
@@ -526,16 +526,15 @@ void ViewGestureController::beginSwipeGesture(WebBackForwardListItem* targetItem
     bool geometryIsFlippedToRoot = layerGeometryFlippedToRoot(snapshotLayerParent);
 
     RetainPtr<CGColorRef> backgroundColor = CGColorGetConstantColor(kCGColorWhite);
-    ViewSnapshot snapshot;
-    if (ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot)) {
-        if (shouldUseSnapshotForSize(snapshot, swipeArea.size(), topContentInset))
-            [m_swipeSnapshotLayer setContents:snapshot.asLayerContents()];
+    if (ViewSnapshot* snapshot = targetItem->snapshot()) {
+        if (shouldUseSnapshotForSize(*snapshot, swipeArea.size(), topContentInset))
+            [m_swipeSnapshotLayer setContents:snapshot->asLayerContents()];
 
-        Color coreColor = snapshot.backgroundColor;
+        Color coreColor = snapshot->backgroundColor();
         if (coreColor.isValid())
             backgroundColor = cachedCGColor(coreColor, ColorSpaceDeviceRGB);
 #if USE_IOSURFACE_VIEW_SNAPSHOTS
-        m_currentSwipeSnapshotSurface = snapshot.surface;
+        m_currentSwipeSnapshotSurface = snapshot->surface();
 #endif
     }
 
@@ -630,10 +629,9 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
         return;
     }
 
-    ViewSnapshot snapshot;
     uint64_t renderTreeSize = 0;
-    if (ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot))
-        renderTreeSize = snapshot.renderTreeSize;
+    if (ViewSnapshot* snapshot = targetItem->snapshot())
+        renderTreeSize = snapshot->renderTreeSize();
 
     m_webPageProxy.process().send(Messages::ViewGestureGeometryCollector::SetRenderTreeSizeNotificationThreshold(renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction), m_webPageProxy.pageID());
 
index 2bc77df0d56c5a593c2b351aba11bcf39e50299d..bed2f189274a2b3b1c3f4e86d3e69704e0470d6c 100644 (file)
 #include <WebCore/Color.h>
 #include <WebCore/IntSize.h>
 #include <WebCore/IOSurface.h>
-#include <chrono>
-#include <wtf/HashMap.h>
+#include <wtf/ListHashSet.h>
 #include <wtf/Noncopyable.h>
+#include <wtf/RefCounted.h>
 #include <wtf/RetainPtr.h>
 #include <wtf/text/WTFString.h>
 
 OBJC_CLASS CAContext;
 
 #if PLATFORM(MAC)
-#define USE_IOSURFACE_VIEW_SNAPSHOTS true
-#define USE_RENDER_SERVER_VIEW_SNAPSHOTS false
+#define USE_IOSURFACE_VIEW_SNAPSHOTS 1
+#define USE_RENDER_SERVER_VIEW_SNAPSHOTS 0
 #else
-#define USE_IOSURFACE_VIEW_SNAPSHOTS false
-#define USE_RENDER_SERVER_VIEW_SNAPSHOTS true
+#define USE_IOSURFACE_VIEW_SNAPSHOTS 0
+#define USE_RENDER_SERVER_VIEW_SNAPSHOTS 1
 #endif
 
+namespace WebCore {
+class IOSurface;
+}
+
 namespace WebKit {
 
+class ViewSnapshotStore;
 class WebBackForwardListItem;
 class WebPageProxy;
 
-struct ViewSnapshot {
+class ViewSnapshot : public RefCounted<ViewSnapshot> {
+public:
 #if USE_IOSURFACE_VIEW_SNAPSHOTS
-    RefPtr<WebCore::IOSurface> surface;
-#endif
-#if USE_RENDER_SERVER_VIEW_SNAPSHOTS
-    uint32_t slotID = 0;
+    static PassRefPtr<ViewSnapshot> create(WebCore::IOSurface*, WebCore::IntSize, size_t imageSizeInBytes);
+#elif USE_RENDER_SERVER_VIEW_SNAPSHOTS
+    static PassRefPtr<ViewSnapshot> create(uint32_t slotID, WebCore::IntSize, size_t imageSizeInBytes);
 #endif
 
-    std::chrono::steady_clock::time_point creationTime;
-    uint64_t renderTreeSize;
-    float deviceScaleFactor;
-    WebCore::IntSize size;
-    size_t imageSizeInBytes = 0;
-    WebCore::Color backgroundColor;
+    ~ViewSnapshot();
 
     void clearImage();
     bool hasImage() const;
     id asLayerContents();
+
+    void setRenderTreeSize(uint64_t renderTreeSize) { m_renderTreeSize = renderTreeSize; }
+    uint64_t renderTreeSize() const { return m_renderTreeSize; }
+
+    void setBackgroundColor(WebCore::Color color) { m_backgroundColor = color; }
+    WebCore::Color backgroundColor() const { return m_backgroundColor; }
+
+    void setDeviceScaleFactor(float deviceScaleFactor) { m_deviceScaleFactor = deviceScaleFactor; }
+    float deviceScaleFactor() const { return m_deviceScaleFactor; }
+
+    size_t imageSizeInBytes() const { return m_imageSizeInBytes; }
+#if USE_IOSURFACE_VIEW_SNAPSHOTS
+    WebCore::IOSurface* surface() const { return m_surface.get(); }
+#endif
+    WebCore::IntSize size() const { return m_size; }
+
+private:
+#if USE_IOSURFACE_VIEW_SNAPSHOTS
+    explicit ViewSnapshot(WebCore::IOSurface*, WebCore::IntSize, size_t imageSizeInBytes);
+#elif USE_RENDER_SERVER_VIEW_SNAPSHOTS
+    explicit ViewSnapshot(uint32_t slotID, WebCore::IntSize, size_t imageSizeInBytes);
+#endif
+
+#if USE_IOSURFACE_VIEW_SNAPSHOTS
+    RefPtr<WebCore::IOSurface> m_surface;
+#elif USE_RENDER_SERVER_VIEW_SNAPSHOTS
+    uint32_t m_slotID;
+#endif
+
+    size_t m_imageSizeInBytes;
+    uint64_t m_renderTreeSize;
+    float m_deviceScaleFactor;
+    WebCore::IntSize m_size;
+    WebCore::Color m_backgroundColor;
 };
 
 class ViewSnapshotStore {
     WTF_MAKE_NONCOPYABLE(ViewSnapshotStore);
+    friend class ViewSnapshot;
 public:
     ViewSnapshotStore();
     ~ViewSnapshotStore();
@@ -79,25 +114,25 @@ public:
     static ViewSnapshotStore& shared();
 
     void recordSnapshot(WebPageProxy&);
-    bool getSnapshot(WebBackForwardListItem*, ViewSnapshot&);
 
     void disableSnapshotting() { m_enabled = false; }
     void enableSnapshotting() { m_enabled = true; }
 
-    void discardSnapshots();
+    void discardSnapshotImages();
 
-#if PLATFORM(IOS)
+#if USE_RENDER_SERVER_VIEW_SNAPSHOTS
     static CAContext *snapshottingContext();
 #endif
 
 private:
+    void didAddImageToSnapshot(ViewSnapshot&);
+    void willRemoveImageFromSnapshot(ViewSnapshot&);
     void pruneSnapshots(WebPageProxy&);
-    void removeSnapshotImage(ViewSnapshot&);
-
-    HashMap<String, ViewSnapshot> m_snapshotMap;
 
     bool m_enabled;
     size_t m_snapshotCacheSize;
+
+    ListHashSet<ViewSnapshot*> m_snapshotsWithImages;
 };
 
 } // namespace WebKit
index 971bff4ea72a927232d7f8cf24f3d880197b666d..55c174a3cf45c7df3038cab81178eafb3402fd98 100644 (file)
@@ -30,7 +30,6 @@
 #import "WebPageProxy.h"
 #import <CoreGraphics/CoreGraphics.h>
 #import <WebCore/IOSurface.h>
-#import <WebCore/UUID.h>
 
 #if PLATFORM(IOS)
 #import <QuartzCore/QuartzCorePrivate.h>
@@ -55,7 +54,7 @@ ViewSnapshotStore::ViewSnapshotStore()
 
 ViewSnapshotStore::~ViewSnapshotStore()
 {
-    discardSnapshots();
+    discardSnapshotImages();
 }
 
 ViewSnapshotStore& ViewSnapshotStore::shared()
@@ -64,7 +63,7 @@ ViewSnapshotStore& ViewSnapshotStore::shared()
     return store;
 }
 
-#if PLATFORM(IOS)
+#if USE_RENDER_SERVER_VIEW_SNAPSHOTS
 CAContext *ViewSnapshotStore::snapshottingContext()
 {
     static CAContext *context;
@@ -82,13 +81,18 @@ CAContext *ViewSnapshotStore::snapshottingContext()
 }
 #endif
 
-void ViewSnapshotStore::removeSnapshotImage(ViewSnapshot& snapshot)
+void ViewSnapshotStore::didAddImageToSnapshot(ViewSnapshot& snapshot)
 {
-    if (!snapshot.hasImage())
-        return;
+    bool isNewEntry = m_snapshotsWithImages.add(&snapshot).isNewEntry;
+    ASSERT_UNUSED(isNewEntry, isNewEntry);
+    m_snapshotCacheSize += snapshot.imageSizeInBytes();
+}
 
-    m_snapshotCacheSize -= snapshot.imageSizeInBytes;
-    snapshot.clearImage();
+void ViewSnapshotStore::willRemoveImageFromSnapshot(ViewSnapshot& snapshot)
+{
+    bool removed = m_snapshotsWithImages.remove(&snapshot);
+    ASSERT_UNUSED(removed, removed);
+    m_snapshotCacheSize -= snapshot.imageSizeInBytes();
 }
 
 void ViewSnapshotStore::pruneSnapshots(WebPageProxy& webPageProxy)
@@ -96,54 +100,11 @@ void ViewSnapshotStore::pruneSnapshots(WebPageProxy& webPageProxy)
     if (m_snapshotCacheSize <= maximumSnapshotCacheSize)
         return;
 
-    uint32_t currentIndex = webPageProxy.backForwardList().currentIndex();
-    uint32_t maxDistance = 0;
-    auto mostDistantSnapshotIter = m_snapshotMap.end();
-    auto backForwardEntries = webPageProxy.backForwardList().entries();
-
-    // First, try to evict the snapshot for the page farthest from the current back-forward item.
-    for (uint32_t i = 0, entryCount = webPageProxy.backForwardList().entries().size(); i < entryCount; i++) {
-        uint32_t distance = std::max(currentIndex, i) - std::min(currentIndex, i);
+    ASSERT(!m_snapshotsWithImages.isEmpty());
 
-        if (i == currentIndex || distance < maxDistance)
-            continue;
-
-        WebBackForwardListItem* item = backForwardEntries[i].get();
-        String snapshotUUID = item->snapshotUUID();
-        if (snapshotUUID.isEmpty())
-            continue;
-
-        const auto& snapshotIter = m_snapshotMap.find(snapshotUUID);
-        if (snapshotIter == m_snapshotMap.end())
-            continue;
-
-        // We're only interested in evicting snapshots that still have images.
-        if (!snapshotIter->value.hasImage())
-            continue;
-
-        mostDistantSnapshotIter = snapshotIter;
-        maxDistance = distance;
-    }
+    // FIXME: We have enough information to do smarter-than-LRU eviction (making use of the back-forward lists, etc.)
 
-    if (mostDistantSnapshotIter != m_snapshotMap.end()) {
-        removeSnapshotImage(mostDistantSnapshotIter->value);
-        return;
-    }
-
-    // If we can't find a most distant item (perhaps because all the snapshots are from
-    // a different WebPageProxy's back-forward list), we should evict the the oldest item.
-    std::chrono::steady_clock::time_point oldestSnapshotTime = std::chrono::steady_clock::time_point::max();
-    String oldestSnapshotUUID;
-
-    for (const auto& uuidAndSnapshot : m_snapshotMap) {
-        if (uuidAndSnapshot.value.creationTime < oldestSnapshotTime && uuidAndSnapshot.value.hasImage()) {
-            oldestSnapshotTime = uuidAndSnapshot.value.creationTime;
-            oldestSnapshotUUID = uuidAndSnapshot.key;
-        }
-    }
-
-    const auto& snapshotIter = m_snapshotMap.find(oldestSnapshotUUID);
-    removeSnapshotImage(snapshotIter->value);
+    m_snapshotsWithImages.first()->clearImage();
 }
 
 void ViewSnapshotStore::recordSnapshot(WebPageProxy& webPageProxy)
@@ -158,77 +119,94 @@ void ViewSnapshotStore::recordSnapshot(WebPageProxy& webPageProxy)
 
     pruneSnapshots(webPageProxy);
 
-    ViewSnapshot snapshot = webPageProxy.takeViewSnapshot();
-    if (!snapshot.hasImage())
+    RefPtr<ViewSnapshot> snapshot = webPageProxy.takeViewSnapshot();
+    if (!snapshot || !snapshot->hasImage())
         return;
 
-    String oldSnapshotUUID = item->snapshotUUID();
-    if (!oldSnapshotUUID.isEmpty()) {
-        const auto& oldSnapshotIter = m_snapshotMap.find(oldSnapshotUUID);
-        if (oldSnapshotIter != m_snapshotMap.end()) {
-            removeSnapshotImage(oldSnapshotIter->value);
-            m_snapshotMap.remove(oldSnapshotIter);
-        }
-    }
-
-    snapshot.creationTime = std::chrono::steady_clock::now();
-    snapshot.renderTreeSize = webPageProxy.renderTreeSize();
-    snapshot.deviceScaleFactor = webPageProxy.deviceScaleFactor();
+    snapshot->setRenderTreeSize(webPageProxy.renderTreeSize());
+    snapshot->setDeviceScaleFactor(webPageProxy.deviceScaleFactor());
+    snapshot->setBackgroundColor(webPageProxy.pageExtendedBackgroundColor());
 
-    item->setSnapshotUUID(createCanonicalUUIDString());
+    item->setSnapshot(snapshot.release());
+}
 
-    m_snapshotMap.add(item->snapshotUUID(), snapshot);
-    m_snapshotCacheSize += snapshot.imageSizeInBytes;
+void ViewSnapshotStore::discardSnapshotImages()
+{
+    while (!m_snapshotsWithImages.isEmpty())
+        m_snapshotsWithImages.first()->clearImage();
 }
 
-bool ViewSnapshotStore::getSnapshot(WebBackForwardListItem* item, ViewSnapshot& snapshot)
+
+#if USE_IOSURFACE_VIEW_SNAPSHOTS
+PassRefPtr<ViewSnapshot> ViewSnapshot::create(IOSurface* surface, IntSize size, size_t imageSizeInBytes)
+{
+    return adoptRef(new ViewSnapshot(surface, size, imageSizeInBytes));
+}
+#elif USE_RENDER_SERVER_VIEW_SNAPSHOTS
+PassRefPtr<ViewSnapshot> ViewSnapshot::create(uint32_t slotID, IntSize size, size_t imageSizeInBytes)
 {
-    if (item->snapshotUUID().isEmpty())
-        return false;
+    return adoptRef(new ViewSnapshot(slotID, size, imageSizeInBytes));
+}
+#endif
 
-    const auto& snapshotIterator = m_snapshotMap.find(item->snapshotUUID());
-    if (snapshotIterator == m_snapshotMap.end())
-        return false;
-    snapshot = snapshotIterator->value;
-    return true;
+#if USE_IOSURFACE_VIEW_SNAPSHOTS
+ViewSnapshot::ViewSnapshot(IOSurface* surface, IntSize size, size_t imageSizeInBytes)
+    : m_surface(surface)
+#elif USE_RENDER_SERVER_VIEW_SNAPSHOTS
+ViewSnapshot::ViewSnapshot(uint32_t slotID, IntSize size, size_t imageSizeInBytes)
+    : m_slotID(slotID)
+#endif
+    , m_imageSizeInBytes(imageSizeInBytes)
+    , m_size(size)
+{
+    if (hasImage())
+        ViewSnapshotStore::shared().didAddImageToSnapshot(*this);
 }
 
-void ViewSnapshotStore::discardSnapshots()
+ViewSnapshot::~ViewSnapshot()
 {
-    for (auto& snapshot : m_snapshotMap.values())
-        removeSnapshotImage(snapshot);
+    clearImage();
 }
 
 bool ViewSnapshot::hasImage() const
 {
-    return imageSizeInBytes;
+#if USE_IOSURFACE_VIEW_SNAPSHOTS
+    return m_surface;
+#elif USE_RENDER_SERVER_VIEW_SNAPSHOTS
+    return m_slotID;
+#endif
 }
 
 void ViewSnapshot::clearImage()
 {
+    if (!hasImage())
+        return;
+
+    ViewSnapshotStore::shared().willRemoveImageFromSnapshot(*this);
+
 #if USE_IOSURFACE_VIEW_SNAPSHOTS
-    surface = nullptr;
+    m_surface = nullptr;
 #elif USE_RENDER_SERVER_VIEW_SNAPSHOTS
-    if (slotID)
-        [ViewSnapshotStore::snapshottingContext() deleteSlot:slotID];
-    slotID = 0;
+    [ViewSnapshotStore::snapshottingContext() deleteSlot:m_slotID];
+    m_slotID = 0;
 #endif
-    imageSizeInBytes = 0;
+    m_imageSizeInBytes = 0;
 }
 
 id ViewSnapshot::asLayerContents()
 {
 #if USE_IOSURFACE_VIEW_SNAPSHOTS
-    if (!surface)
+    if (!m_surface)
         return nullptr;
 
-    // FIXME: This should destroy the surface and inform the ViewSnapshotStore to reduce m_snapshotCacheSize.
-    if (surface->setIsVolatile(false) != IOSurface::SurfaceState::Valid)
+    if (m_surface->setIsVolatile(false) != IOSurface::SurfaceState::Valid) {
+        clearImage();
         return nullptr;
+    }
 
-    return (id)surface->surface();
+    return (id)m_surface->surface();
 #elif USE_RENDER_SERVER_VIEW_SNAPSHOTS
-    return [CAContext objectForSlot:slotID];
+    return [CAContext objectForSlot:m_slotID];
 #endif
 }