- Revision
- 167775
- Author
- [email protected]
- Date
- 2014-04-24 14:45:55 -0700 (Thu, 24 Apr 2014)
Log Message
WebKit2 View Gestures: Use a single struct for the snapshot, and pass it around
https://bugs.webkit.org/show_bug.cgi?id=132114
Reviewed by Simon Fraser.
Have only a single map in ViewSnapshotStore, from back-forward item
to ViewSnapshotStore::Snapshot, and return the Snapshot struct when looking
up snapshots (via getSnapshot()), so that future patches can persist additional
information along with the snapshot.
* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::beginSwipeGesture):
(WebKit::ViewGestureController::endSwipeGesture):
* UIProcess/mac/ViewGestureController.h:
* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::retrieveSnapshotForItem):
(WebKit::ViewGestureController::beginSwipeGesture):
(WebKit::ViewGestureController::endSwipeGesture):
Adopt getSnapshot() instead of snapshotAndRenderTreeSize().
Move retrieveSnapshotForItem out into a separate function (for future use).
* UIProcess/mac/ViewSnapshotStore.h:
(WebKit::ViewSnapshotStore::disableSnapshotting):
(WebKit::ViewSnapshotStore::enableSnapshotting):
* UIProcess/mac/ViewSnapshotStore.mm:
(WebKit::ViewSnapshotStore::pruneSnapshots):
(WebKit::ViewSnapshotStore::recordSnapshot):
(WebKit::ViewSnapshotStore::getSnapshot):
(WebKit::ViewSnapshotStore::snapshotAndRenderTreeSize): Deleted.
Make Snapshot struct public.
Get rid of the separate map of back-forward items to render tree sizes.
When evicting, instead of removing the entry, clear out its snapshot image;
this way, we can keep other snapshot metadata around.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (167774 => 167775)
--- trunk/Source/WebKit2/ChangeLog 2014-04-24 21:44:42 UTC (rev 167774)
+++ trunk/Source/WebKit2/ChangeLog 2014-04-24 21:45:55 UTC (rev 167775)
@@ -1,3 +1,39 @@
+2014-04-24 Tim Horton <[email protected]>
+
+ WebKit2 View Gestures: Use a single struct for the snapshot, and pass it around
+ https://bugs.webkit.org/show_bug.cgi?id=132114
+
+ Reviewed by Simon Fraser.
+
+ Have only a single map in ViewSnapshotStore, from back-forward item
+ to ViewSnapshotStore::Snapshot, and return the Snapshot struct when looking
+ up snapshots (via getSnapshot()), so that future patches can persist additional
+ information along with the snapshot.
+
+ * UIProcess/ios/ViewGestureControllerIOS.mm:
+ (WebKit::ViewGestureController::beginSwipeGesture):
+ (WebKit::ViewGestureController::endSwipeGesture):
+ * UIProcess/mac/ViewGestureController.h:
+ * UIProcess/mac/ViewGestureControllerMac.mm:
+ (WebKit::ViewGestureController::retrieveSnapshotForItem):
+ (WebKit::ViewGestureController::beginSwipeGesture):
+ (WebKit::ViewGestureController::endSwipeGesture):
+ Adopt getSnapshot() instead of snapshotAndRenderTreeSize().
+ Move retrieveSnapshotForItem out into a separate function (for future use).
+
+ * UIProcess/mac/ViewSnapshotStore.h:
+ (WebKit::ViewSnapshotStore::disableSnapshotting):
+ (WebKit::ViewSnapshotStore::enableSnapshotting):
+ * UIProcess/mac/ViewSnapshotStore.mm:
+ (WebKit::ViewSnapshotStore::pruneSnapshots):
+ (WebKit::ViewSnapshotStore::recordSnapshot):
+ (WebKit::ViewSnapshotStore::getSnapshot):
+ (WebKit::ViewSnapshotStore::snapshotAndRenderTreeSize): Deleted.
+ Make Snapshot struct public.
+ Get rid of the separate map of back-forward items to render tree sizes.
+ When evicting, instead of removing the entry, clear out its snapshot image;
+ this way, we can keep other snapshot metadata around.
+
2014-04-24 Enrica Casucci <[email protected]>
[iOS WebKit2] Should properly handle focus redirect (keyboard state changes when focus changes).
Modified: trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm (167774 => 167775)
--- trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm 2014-04-24 21:44:42 UTC (rev 167774)
+++ trunk/Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm 2014-04-24 21:45:55 UTC (rev 167775)
@@ -151,18 +151,18 @@
WebKit::WebBackForwardListItem* targetItem = direction == SwipeDirection::Left ? m_webPageProxy.backForwardList().backItem() : m_webPageProxy.backForwardList().forwardItem();
- auto snapshot = WebKit::ViewSnapshotStore::shared().snapshotAndRenderTreeSize(targetItem).first;
-
RetainPtr<UIViewController> snapshotViewController = adoptNS([[UIViewController alloc] init]);
m_snapshotView = adoptNS([[UIView alloc] initWithFrame:[m_liveSwipeView frame]]);
- if (snapshot) {
+
+ ViewSnapshotStore::Snapshot snapshot;
+ if (ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot)) {
#if USE(IOSURFACE)
- if (snapshot->setIsVolatile(false) == IOSurface::SurfaceState::Valid) {
- [m_snapshotView layer].contents = (id)snapshot->surface();
- m_currentSwipeSnapshotSurface = snapshot;
+ if (snapshot.surface->setIsVolatile(false) == IOSurface::SurfaceState::Valid) {
+ [m_snapshotView layer].contents = (id)snapshot.surface->surface();
+ m_currentSwipeSnapshotSurface = snapshot.surface;
}
#else
- [m_snapshotView layer].contents = (id)snapshot.get();
+ [m_snapshotView layer].contents = (id)snapshot.image.get();
#endif
}
[m_snapshotView setBackgroundColor:[UIColor whiteColor]];
@@ -223,9 +223,12 @@
removeSwipeSnapshot();
return;
}
-
- m_targetRenderTreeSize = ViewSnapshotStore::shared().snapshotAndRenderTreeSize(targetItem).second * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
-
+
+ ViewSnapshotStore::Snapshot snapshot;
+ m_targetRenderTreeSize = 0;
+ if (ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot))
+ m_targetRenderTreeSize = 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
// displaying the destination item's snapshot.
Modified: trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h (167774 => 167775)
--- trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h 2014-04-24 21:44:42 UTC (rev 167774)
+++ trunk/Source/WebKit2/UIProcess/mac/ViewGestureController.h 2014-04-24 21:45:55 UTC (rev 167775)
@@ -130,6 +130,7 @@
void endSwipeGesture(WebBackForwardListItem* targetItem, bool cancelled);
bool deltaIsSufficientToBeginSwipe(NSEvent *);
bool scrollEventCanBecomeSwipe(NSEvent *, SwipeDirection&);
+ WebCore::IOSurface* retrieveSnapshotForItem(WebBackForwardListItem*);
#endif
WebPageProxy& m_webPageProxy;
Modified: trunk/Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm (167774 => 167775)
--- trunk/Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm 2014-04-24 21:44:42 UTC (rev 167774)
+++ trunk/Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm 2014-04-24 21:45:55 UTC (rev 167775)
@@ -426,6 +426,21 @@
return liveLayerPathsFromRoot[0][shortestPathLength];
}
+IOSurface* ViewGestureController::retrieveSnapshotForItem(WebBackForwardListItem* targetItem)
+{
+ ViewSnapshotStore::Snapshot snapshot;
+ if (!ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot))
+ return nullptr;
+
+ if (!snapshot.surface)
+ return nullptr;
+
+ if (snapshot.surface->setIsVolatile(false) != IOSurface::SurfaceState::Valid)
+ return nullptr;
+
+ return snapshot.surface.get();
+}
+
void ViewGestureController::beginSwipeGesture(WebBackForwardListItem* targetItem, SwipeDirection direction)
{
ASSERT(m_currentSwipeLiveLayers.isEmpty());
@@ -437,9 +452,9 @@
m_swipeSnapshotLayer = adoptNS([[CALayer alloc] init]);
[m_swipeSnapshotLayer setBackgroundColor:CGColorGetConstantColor(kCGColorWhite)];
- RefPtr<IOSurface> snapshot = ViewSnapshotStore::shared().snapshotAndRenderTreeSize(targetItem).first;
+ IOSurface* snapshot = retrieveSnapshotForItem(targetItem);
- if (snapshot && snapshot->setIsVolatile(false) == IOSurface::SurfaceState::Valid) {
+ if (snapshot) {
m_currentSwipeSnapshotSurface = snapshot;
[m_swipeSnapshotLayer setContents:(id)snapshot->surface()];
}
@@ -557,7 +572,11 @@
return;
}
- uint64_t renderTreeSize = ViewSnapshotStore::shared().snapshotAndRenderTreeSize(targetItem).second;
+ ViewSnapshotStore::Snapshot snapshot;
+ uint64_t renderTreeSize = 0;
+ if (ViewSnapshotStore::shared().getSnapshot(targetItem, snapshot))
+ renderTreeSize = snapshot.renderTreeSize;
+
m_webPageProxy.process().send(Messages::ViewGestureGeometryCollector::SetRenderTreeSizeNotificationThreshold(renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction), m_webPageProxy.pageID());
// We don't want to replace the current back-forward item's snapshot
Modified: trunk/Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h (167774 => 167775)
--- trunk/Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h 2014-04-24 21:44:42 UTC (rev 167774)
+++ trunk/Source/WebKit2/UIProcess/mac/ViewSnapshotStore.h 2014-04-24 21:45:55 UTC (rev 167775)
@@ -46,19 +46,6 @@
static ViewSnapshotStore& shared();
- void recordSnapshot(WebPageProxy&);
-#if USE(IOSURFACE)
- std::pair<RefPtr<WebCore::IOSurface>, uint64_t> snapshotAndRenderTreeSize(WebBackForwardListItem*);
-#else
- std::pair<RetainPtr<CGImageRef>, uint64_t> snapshotAndRenderTreeSize(WebBackForwardListItem*);
-#endif
-
- void disableSnapshotting() { m_enabled = false; }
- void enableSnapshotting() { m_enabled = true; }
-
-private:
- void pruneSnapshots(WebPageProxy&);
-
struct Snapshot {
#if USE(IOSURFACE)
RefPtr<WebCore::IOSurface> surface;
@@ -67,10 +54,22 @@
#endif
std::chrono::steady_clock::time_point creationTime;
+ uint64_t renderTreeSize;
+
+ void clearImage();
+ bool hasImage() const;
};
+ void recordSnapshot(WebPageProxy&);
+ bool getSnapshot(WebBackForwardListItem*, Snapshot&);
+
+ void disableSnapshotting() { m_enabled = false; }
+ void enableSnapshotting() { m_enabled = true; }
+
+private:
+ void pruneSnapshots(WebPageProxy&);
+
HashMap<String, Snapshot> m_snapshotMap;
- HashMap<String, uint64_t> m_renderTreeSizeMap;
bool m_enabled;
};
Modified: trunk/Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm (167774 => 167775)
--- trunk/Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm 2014-04-24 21:44:42 UTC (rev 167774)
+++ trunk/Source/WebKit2/UIProcess/mac/ViewSnapshotStore.mm 2014-04-24 21:45:55 UTC (rev 167775)
@@ -72,14 +72,24 @@
WebBackForwardListItem* item = backForwardEntries[i].get();
String snapshotUUID = item->snapshotUUID();
- if (!snapshotUUID.isEmpty() && m_snapshotMap.contains(snapshotUUID)) {
- mostDistantSnapshottedItem = item;
- maxDistance = distance;
- }
+ if (snapshotUUID.isEmpty())
+ continue;
+
+ const auto& snapshot = m_snapshotMap.find(mostDistantSnapshottedItem->snapshotUUID());
+ if (snapshot == m_snapshotMap.end())
+ continue;
+
+ // We're only interested in evicting snapshots that still have images.
+ if (!snapshot->value.hasImage())
+ continue;
+
+ mostDistantSnapshottedItem = item;
+ maxDistance = distance;
}
if (mostDistantSnapshottedItem) {
- m_snapshotMap.remove(mostDistantSnapshottedItem->snapshotUUID());
+ const auto& snapshot = m_snapshotMap.find(mostDistantSnapshottedItem->snapshotUUID());
+ snapshot->value.clearImage();
return;
}
@@ -95,7 +105,8 @@
}
}
- m_snapshotMap.remove(oldestSnapshotUUID);
+ const auto& snapshot = m_snapshotMap.find(oldestSnapshotUUID);
+ snapshot->value.clearImage();
}
#if USE(IOSURFACE)
@@ -130,15 +141,14 @@
pruneSnapshots(webPageProxy);
String oldSnapshotUUID = item->snapshotUUID();
- if (!oldSnapshotUUID.isEmpty()) {
+ if (!oldSnapshotUUID.isEmpty())
m_snapshotMap.remove(oldSnapshotUUID);
- m_renderTreeSizeMap.remove(oldSnapshotUUID);
- }
item->setSnapshotUUID(createCanonicalUUIDString());
Snapshot snapshot;
snapshot.creationTime = std::chrono::steady_clock::now();
+ snapshot.renderTreeSize = webPageProxy.renderTreeSize();
#if USE(IOSURFACE)
snapshot.surface = createIOSurfaceFromImage(snapshotImage.get());
@@ -148,31 +158,35 @@
#endif
m_snapshotMap.add(item->snapshotUUID(), snapshot);
- m_renderTreeSizeMap.add(item->snapshotUUID(), webPageProxy.renderTreeSize());
}
-#if USE(IOSURFACE)
-std::pair<RefPtr<IOSurface>, uint64_t> ViewSnapshotStore::snapshotAndRenderTreeSize(WebBackForwardListItem* item)
-#else
-std::pair<RetainPtr<CGImageRef>, uint64_t> ViewSnapshotStore::snapshotAndRenderTreeSize(WebBackForwardListItem* item)
-#endif
+bool ViewSnapshotStore::getSnapshot(WebBackForwardListItem* item, ViewSnapshotStore::Snapshot& snapshot)
{
if (item->snapshotUUID().isEmpty())
- return std::make_pair(nullptr, 0);
+ return false;
- const auto& renderTreeSize = m_renderTreeSizeMap.find(item->snapshotUUID());
- if (renderTreeSize == m_renderTreeSizeMap.end())
- return std::make_pair(nullptr, 0);
+ const auto& snapshotIterator = m_snapshotMap.find(item->snapshotUUID());
+ if (snapshotIterator == m_snapshotMap.end())
+ return false;
+ snapshot = snapshotIterator->value;
+ return true;
+}
- const auto& snapshot = m_snapshotMap.find(item->snapshotUUID());
+void ViewSnapshotStore::Snapshot::clearImage()
+{
+#if USE(IOSURFACE)
+ surface = nullptr;
+#else
+ image = nullptr;
+#endif
+}
- if (snapshot == m_snapshotMap.end())
- return std::make_pair(nullptr, renderTreeSize->value);
-
+bool ViewSnapshotStore::Snapshot::hasImage() const
+{
#if USE(IOSURFACE)
- return std::make_pair(snapshot->value.surface, renderTreeSize->value);
+ return surface;
#else
- return std::make_pair(snapshot->value.image, renderTreeSize->value);
+ return image;
#endif
}