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 5565eef..909234d 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 76e41c2..8d19c66 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 0960c79..feac19c 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 37cfe7b..85c49e9 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 cf90a15..4aa9797 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 11dd711..d895427 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 31f2541..7ffa9d7 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 7ca46e6..39eeb13 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 1f15641..3d98714 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 bd956c5..42177c1 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 22ecdf8..3a31996 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 2a0cdf6..64fe0c8 100644 (file)
@@ -470,7 +470,7 @@ LayerOrView *PageClientImpl::acceleratedCompositingRootLayer() const
     return nullptr;
 }
 
-ViewSnapshot PageClientImpl::takeViewSnapshot()
+PassRefPtr<ViewSnapshot> PageClientImpl::takeViewSnapshot()
 {
     return [m_webView _takeViewSnapshot];
 }
index 22429fd..914a847 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 7d4999a..523d0c8 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 611fa97..f57c816 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 47a8746..e1b5a02 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 b6085a2..8e12d12 100644 (file)
@@ -50,7 +50,7 @@ class IOSurface;
 
 namespace WebKit {
 
-struct ViewSnapshot;
+class ViewSnapshot;
 class WebBackForwardListItem;
 class WebPageProxy;
 
index 1549291..b96a59d 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 2bc77df..bed2f18 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 971bff4..55c174a 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
 }