REGRESSION (r164337): Pages are sometimes cut off/oriented incorrectly after using...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Aug 2014 23:32:24 +0000 (23:32 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Aug 2014 23:32:24 +0000 (23:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135622
<rdar://problem/17202556>

Reviewed by Dan Bernstein.

In some cases (when the page changed scroll offset while thumbnailed),
when transitioning back to thumbnail scale = 1, we would get the math
wrong and end up with a non-identity sublayerTransform on the DrawingArea.

Luckily, none of this code is necessary anymore, as the only client
of WKThumbnailView only uses its snapshotting mode.

* Shared/ImageOptions.h:
Remove SnapshotOptionsRespectDrawingAreaTransform; DrawingArea no longer
has a rootLayerTransform().

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::setThumbnailScale): Deleted.
* UIProcess/WebPageProxy.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::WebPage):
(WebKit::WebPage::scaledSnapshotWithOptions):
(WebKit::WebPage::snapshotAtSize):

(WebKit::WebPage::setThumbnailScale): Deleted.
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
Remove setThumbnailScale and SnapshotOptionsRespectDrawingAreaTransform.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didCommitLoad):
Revert this to its state before r164337, as we no longer have "thumbnail scale".

* UIProcess/API/Cocoa/_WKThumbnailView.h:
* UIProcess/API/Cocoa/_WKThumbnailView.mm:
(-[_WKThumbnailView initWithFrame:fromWKView:]):
(-[_WKThumbnailView _viewWasUnparented]):
(-[_WKThumbnailView _viewWasParented]):
(-[_WKThumbnailView _requestSnapshotIfNeeded]):
(-[_WKThumbnailView setScale:]):
Clean up code assuming _shouldApplyThumbnailScale = NO, _usesSnapshot = YES.

(-[_WKThumbnailView setUsesSnapshot:]):
(-[_WKThumbnailView usesSnapshot]):
Always return YES from usesSnapshot; we only support snapshotting WKThumbnailViews.
Ignore setUsesSnapshot.

* UIProcess/API/mac/WKView.mm:
(-[WKView _setThumbnailView:]):
(-[WKView _updateThumbnailViewLayer]):
Stop checking usesSnapshot; it's always true.

* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
* WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
(WebKit::TiledCoreAnimationDrawingArea::setRootLayerTransform): Deleted.
* WebProcess/WebPage/DrawingArea.cpp:
(WebKit::DrawingArea::rootLayerTransform): Deleted.
* WebProcess/WebPage/DrawingArea.h:
(WebKit::DrawingArea::setRootLayerTransform): Deleted.
Remove rootLayerTransform() and setRootLayerTransform().

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

14 files changed:
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/ImageOptions.h
Source/WebKit2/UIProcess/API/Cocoa/_WKThumbnailView.h
Source/WebKit2/UIProcess/API/Cocoa/_WKThumbnailView.mm
Source/WebKit2/UIProcess/API/mac/WKView.mm
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/WebProcess/WebPage/DrawingArea.cpp
Source/WebKit2/WebProcess/WebPage/DrawingArea.h
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.h
Source/WebKit2/WebProcess/WebPage/WebPage.messages.in
Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h
Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

index b46e4ec..aef8f81 100644 (file)
@@ -1,3 +1,67 @@
+2014-08-05  Tim Horton  <timothy_horton@apple.com>
+
+        REGRESSION (r164337): Pages are sometimes cut off/oriented incorrectly after using WKThumbnailView
+        https://bugs.webkit.org/show_bug.cgi?id=135622
+        <rdar://problem/17202556>
+
+        Reviewed by Dan Bernstein.
+
+        In some cases (when the page changed scroll offset while thumbnailed),
+        when transitioning back to thumbnail scale = 1, we would get the math
+        wrong and end up with a non-identity sublayerTransform on the DrawingArea.
+
+        Luckily, none of this code is necessary anymore, as the only client
+        of WKThumbnailView only uses its snapshotting mode.
+
+        * Shared/ImageOptions.h:
+        Remove SnapshotOptionsRespectDrawingAreaTransform; DrawingArea no longer
+        has a rootLayerTransform().
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::setThumbnailScale): Deleted.
+        * UIProcess/WebPageProxy.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::WebPage):
+        (WebKit::WebPage::scaledSnapshotWithOptions):
+        (WebKit::WebPage::snapshotAtSize):
+        
+        (WebKit::WebPage::setThumbnailScale): Deleted.
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        Remove setThumbnailScale and SnapshotOptionsRespectDrawingAreaTransform.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::didCommitLoad):
+        Revert this to its state before r164337, as we no longer have "thumbnail scale".
+
+        * UIProcess/API/Cocoa/_WKThumbnailView.h:
+        * UIProcess/API/Cocoa/_WKThumbnailView.mm:
+        (-[_WKThumbnailView initWithFrame:fromWKView:]):
+        (-[_WKThumbnailView _viewWasUnparented]):
+        (-[_WKThumbnailView _viewWasParented]):
+        (-[_WKThumbnailView _requestSnapshotIfNeeded]):
+        (-[_WKThumbnailView setScale:]):
+        Clean up code assuming _shouldApplyThumbnailScale = NO, _usesSnapshot = YES.
+
+        (-[_WKThumbnailView setUsesSnapshot:]):
+        (-[_WKThumbnailView usesSnapshot]):
+        Always return YES from usesSnapshot; we only support snapshotting WKThumbnailViews.
+        Ignore setUsesSnapshot.
+
+        * UIProcess/API/mac/WKView.mm:
+        (-[WKView _setThumbnailView:]):
+        (-[WKView _updateThumbnailViewLayer]):
+        Stop checking usesSnapshot; it's always true.
+
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h:
+        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
+        (WebKit::TiledCoreAnimationDrawingArea::setRootLayerTransform): Deleted.
+        * WebProcess/WebPage/DrawingArea.cpp:
+        (WebKit::DrawingArea::rootLayerTransform): Deleted.
+        * WebProcess/WebPage/DrawingArea.h:
+        (WebKit::DrawingArea::setRootLayerTransform): Deleted.
+        Remove rootLayerTransform() and setRootLayerTransform().
+
 2014-08-05  Peyton Randolph  <prandolph@apple.com>
 
         Rename MAC_LONG_PRESS feature flag to LONG_MOUSE_PRESS.
index 9de9445..8b66c19 100644 (file)
@@ -37,7 +37,6 @@ enum {
     SnapshotOptionsExcludeSelectionHighlighting = 1 << 1,
     SnapshotOptionsInViewCoordinates = 1 << 2,
     SnapshotOptionsPaintSelectionRectangle = 1 << 3,
-    SnapshotOptionsRespectDrawingAreaTransform = 1 << 4,
     SnapshotOptionsExcludeDeviceScaleFactor = 1 << 5,
 };
 typedef uint32_t SnapshotOptions;
index 4889594..6735aab 100644 (file)
@@ -39,6 +39,8 @@ WK_CLASS_AVAILABLE(10_10, 8_0)
 - (instancetype)initWithFrame:(NSRect)frame fromWKView:(WKView *)wkView;
 
 @property (nonatomic) CGFloat scale;
+
+// This should be removed when all clients go away; it is always YES now.
 @property (nonatomic) BOOL usesSnapshot;
 
 @end
index e765f34..4708d65 100644 (file)
@@ -52,8 +52,6 @@ using namespace WebKit;
     BOOL _originalMayStartMediaWhenInWindow;
     BOOL _originalSourceViewIsInWindow;
 
-    BOOL _shouldApplyThumbnailScale;
-
     BOOL _snapshotWasDeferred;
     double _lastSnapshotScale;
 }
@@ -76,16 +74,11 @@ using namespace WebKit;
     _originalMayStartMediaWhenInWindow = _webPageProxy->mayStartMediaWhenInWindow();
     _originalSourceViewIsInWindow = !![_wkView window];
 
-    _shouldApplyThumbnailScale = !_originalSourceViewIsInWindow;
-
     return self;
 }
 
 - (void)_viewWasUnparented
 {
-    if (_shouldApplyThumbnailScale)
-        _webPageProxy->setThumbnailScale(1);
-
     [_wkView _setThumbnailView:nil];
 
     self.layer.contents = nil;
@@ -99,9 +92,6 @@ using namespace WebKit;
     if ([_wkView _thumbnailView])
         return;
 
-    if (_shouldApplyThumbnailScale)
-        _webPageProxy->setThumbnailScale(_scale);
-
     if (!_originalSourceViewIsInWindow)
         _webPageProxy->setMayStartMediaWhenInWindow(false);
 
@@ -111,9 +101,6 @@ using namespace WebKit;
 
 - (void)_requestSnapshotIfNeeded
 {
-    if (!_usesSnapshot)
-        return;
-
     if (self.layer.contents && _lastSnapshotScale == _scale)
         return;
 
@@ -126,7 +113,7 @@ using namespace WebKit;
 
     RetainPtr<_WKThumbnailView> thumbnailView = self;
     IntRect snapshotRect(IntPoint(), _webPageProxy->viewSize() - IntSize(0, _webPageProxy->topContentInset()));
-    SnapshotOptions options = SnapshotOptionsRespectDrawingAreaTransform | SnapshotOptionsInViewCoordinates;
+    SnapshotOptions options = SnapshotOptionsInViewCoordinates;
     IntSize bitmapSize = snapshotRect.size();
     bitmapSize.scale(_scale * _webPageProxy->deviceScaleFactor());
     _lastSnapshotScale = _scale;
@@ -166,31 +153,19 @@ using namespace WebKit;
 
     _scale = scale;
 
-    if (self.window && _shouldApplyThumbnailScale)
-        _webPageProxy->setThumbnailScale(_scale);
-
-    if (_usesSnapshot)
-        [self _requestSnapshotIfNeeded];
+    [self _requestSnapshotIfNeeded];
 
     self.layer.sublayerTransform = CATransform3DMakeScale(_scale, _scale, 1);
 }
 
+// This should be removed when all clients go away; it is always YES now.
 - (void)setUsesSnapshot:(BOOL)usesSnapshot
 {
-    if (_usesSnapshot == usesSnapshot)
-        return;
-
-    _usesSnapshot = usesSnapshot;
-
-    if (!self.window)
-        return;
+}
 
-    if (usesSnapshot)
-        [self _requestSnapshotIfNeeded];
-    else {
-        _webPageProxy->setThumbnailScale(_scale);
-        [_wkView _reparentLayerTreeInThumbnailView];
-    }
+- (BOOL)usesSnapshot
+{
+    return YES;
 }
 
 - (void)_setThumbnailLayer:(CALayer *)layer
index 5c9768a..e686b8d 100644 (file)
@@ -3547,9 +3547,6 @@ static NSString *pathWithUniqueFilenameForPath(NSString *path)
         [self _updateThumbnailViewLayer];
     else
         [self _setAcceleratedCompositingModeRootLayer:_data->_rootLayer.get()];
-
-    if (!thumbnailView.usesSnapshot)
-        _data->_page->viewStateDidChange(ViewState::WindowIsActive | ViewState::IsInWindow | ViewState::IsVisible);
 }
 
 - (_WKThumbnailView *)_thumbnailView
@@ -3562,7 +3559,7 @@ static NSString *pathWithUniqueFilenameForPath(NSString *path)
     _WKThumbnailView *thumbnailView = _data->_thumbnailView;
     ASSERT(thumbnailView);
 
-    if (!thumbnailView.usesSnapshot || (thumbnailView._waitingForSnapshot && self.window))
+    if (thumbnailView._waitingForSnapshot && self.window)
         [self _reparentLayerTreeInThumbnailView];
 }
 
index 89a60fc..0e560ab 100644 (file)
@@ -5052,14 +5052,6 @@ void WebPageProxy::unwrapCryptoKey(const Vector<uint8_t>& wrappedKey, bool& succ
 }
 #endif
 
-void WebPageProxy::setThumbnailScale(double scale)
-{
-    if (!isValid())
-        return;
-
-    m_process->send(Messages::WebPage::SetThumbnailScale(scale), m_pageID);
-}
-
 void WebPageProxy::addMIMETypeWithCustomContentProvider(const String& mimeType)
 {
     m_process->send(Messages::WebPage::AddMIMETypeWithCustomContentProvider(mimeType), m_pageID);
index a64b935..965f3c8 100644 (file)
@@ -901,8 +901,6 @@ public:
     void unwrapCryptoKey(const Vector<uint8_t>&, bool& succeeded, Vector<uint8_t>&);
 #endif
 
-    void setThumbnailScale(double);
-
     void takeSnapshot(WebCore::IntRect, WebCore::IntSize bitmapSize, SnapshotOptions, std::function<void (const ShareableBitmap::Handle&, CallbackBase::Error)>);
 
     void navigationGestureDidBegin();
index 8c8e3cb..41463b9 100644 (file)
@@ -87,11 +87,6 @@ void DrawingArea::dispatchAfterEnsuringUpdatedScrollPosition(std::function<void
     function();
 }
 
-TransformationMatrix DrawingArea::rootLayerTransform() const
-{
-    return TransformationMatrix();
-}
-
 #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
 PassRefPtr<WebCore::DisplayRefreshMonitor> DrawingArea::createDisplayRefreshMonitor(PlatformDisplayID)
 {
index 06528ea..1a62793 100644 (file)
@@ -107,9 +107,6 @@ public:
     virtual PassRefPtr<WebCore::DisplayRefreshMonitor> createDisplayRefreshMonitor(PlatformDisplayID);
 #endif
 
-    virtual WebCore::TransformationMatrix rootLayerTransform() const;
-    virtual void setRootLayerTransform(const WebCore::TransformationMatrix&) { }
-
 #if USE(COORDINATED_GRAPHICS)
     virtual void didReceiveCoordinatedLayerTreeHostMessage(IPC::Connection*, IPC::MessageDecoder&) = 0;
 #endif
index 096f741..f38e477 100644 (file)
@@ -320,8 +320,6 @@ WebPage::WebPage(uint64_t pageID, const WebPageCreationParameters& parameters)
     , m_viewState(parameters.viewState)
     , m_processSuppressionDisabledByWebPreference("Process suppression is disabled.")
     , m_pendingNavigationID(0)
-    , m_pageScaleWithoutThumbnailScale(1)
-    , m_thumbnailScale(1)
 #if ENABLE(WEBGL)
     , m_systemWebGLPolicy(WebGLAllowCreation)
 #endif
@@ -1633,9 +1631,6 @@ void WebPage::takeSnapshot(IntRect snapshotRect, IntSize bitmapSize, uint32_t op
 PassRefPtr<WebImage> WebPage::scaledSnapshotWithOptions(const IntRect& rect, double additionalScaleFactor, SnapshotOptions options)
 {
     IntRect snapshotRect = rect;
-    if (options & SnapshotOptionsRespectDrawingAreaTransform)
-        snapshotRect = m_drawingArea->rootLayerTransform().inverse().mapRect(snapshotRect);
-
     IntSize bitmapSize = snapshotRect.size();
     double scaleFactor = additionalScaleFactor;
     if (!(options & SnapshotOptionsExcludeDeviceScaleFactor))
@@ -1656,9 +1651,6 @@ PassRefPtr<WebImage> WebPage::snapshotAtSize(const IntRect& rect, const IntSize&
         return nullptr;
 
     IntRect snapshotRect = rect;
-    if (options & SnapshotOptionsRespectDrawingAreaTransform)
-        snapshotRect = m_drawingArea->rootLayerTransform().inverse().mapRect(snapshotRect);
-
     float horizontalScaleFactor = static_cast<float>(bitmapSize.width()) / rect.width();
     float verticalScaleFactor = static_cast<float>(bitmapSize.height()) / rect.height();
     float scaleFactor = std::max(horizontalScaleFactor, verticalScaleFactor);
@@ -4458,13 +4450,8 @@ void WebPage::didCommitLoad(WebFrame* frame)
     if (frame->coreFrame()->loader().loadType() == FrameLoadType::Standard) {
         Page* page = frame->coreFrame()->page();
 
-        if (page) {
-            if (m_thumbnailScale != 1) {
-                m_pageScaleWithoutThumbnailScale = 1;
-                setThumbnailScale(m_thumbnailScale);
-            } else if (page->pageScaleFactor() != 1)
-                scalePage(1, IntPoint());
-        }
+        if (page && page->pageScaleFactor() != 1)
+            scalePage(1, IntPoint());
     }
 #if PLATFORM(IOS)
     m_hasReceivedVisibleContentRectsAfterDidCommitLoad = false;
@@ -4751,40 +4738,6 @@ PassRefPtr<DocumentLoader> WebPage::createDocumentLoader(Frame& frame, const Res
     return documentLoader.release();
 }
 
-void WebPage::setThumbnailScale(double thumbnailScale)
-{
-    // FIXME (129014): If the page programmatically scales while thumbnailed, we will restore the wrong scroll position.
-
-    ASSERT_ARG(thumbnailScale, thumbnailScale > 0);
-
-    double currentPageScaleFactor = pageScaleFactor();
-
-    if (thumbnailScale == m_thumbnailScale && currentPageScaleFactor == m_thumbnailScale * m_pageScaleWithoutThumbnailScale)
-        return;
-
-    if (m_thumbnailScale == 1) {
-        m_pageScaleWithoutThumbnailScale = currentPageScaleFactor;
-        m_scrollPositionIgnoringThumbnailScale = m_page->mainFrame().view()->scrollPosition();
-    }
-
-    m_thumbnailScale = thumbnailScale;
-
-    // Scale the page, but leave the original page scale intact if there was any.
-    scalePage(m_thumbnailScale * m_pageScaleWithoutThumbnailScale, IntPoint());
-
-    // Scroll as far as we can towards the original scroll position in the scaled page.
-    // This may get constrained; we'll transform the drawing area to expose the right part of the page.
-    m_page->mainFrame().view()->setScrollPosition(IntPoint(m_scrollPositionIgnoringThumbnailScale.x() * m_thumbnailScale, m_scrollPositionIgnoringThumbnailScale.y() * m_thumbnailScale));
-
-    double inverseScale = 1 / m_thumbnailScale;
-    IntPoint newScrollPosition = m_page->mainFrame().view()->scrollPosition();
-    TransformationMatrix transform;
-    transform.translate((newScrollPosition.x() * inverseScale) - m_scrollPositionIgnoringThumbnailScale.x(), (newScrollPosition.y() * inverseScale) - m_scrollPositionIgnoringThumbnailScale.y());
-    transform.scale(inverseScale);
-
-    drawingArea()->setRootLayerTransform(transform);
-}
-
 void WebPage::getBytecodeProfile(uint64_t callbackID)
 {
     ASSERT(JSDOMWindow::commonVM().m_perBytecodeProfiler);
index 6cdf70a..88a9615 100644 (file)
@@ -391,8 +391,6 @@ public:
     void addPluginView(PluginView*);
     void removePluginView(PluginView*);
 
-    void setThumbnailScale(double);
-
     bool isVisible() const { return m_viewState & WebCore::ViewState::IsVisible; }
     bool isVisibleOrOccluded() const { return m_viewState & WebCore::ViewState::IsVisibleOrOccluded; }
 
@@ -1273,10 +1271,6 @@ private:
 
     uint64_t m_pendingNavigationID;
 
-    double m_pageScaleWithoutThumbnailScale;
-    WebCore::IntPoint m_scrollPositionIgnoringThumbnailScale;
-    double m_thumbnailScale;
-
 #if ENABLE(WEBGL)
     WebCore::WebGLLoadPolicy m_systemWebGLPolicy;
 #endif
index 3f84a68..ebf720d 100644 (file)
@@ -379,8 +379,6 @@ messages -> WebPage LegacyReceiver {
   
     SetScrollPinningBehavior(uint32_t pinning)
 
-    SetThumbnailScale(double scale)
-
     GetBytecodeProfile(uint64_t callbackID)
     
     TakeSnapshot(WebCore::IntRect snapshotRect, WebCore::IntSize bitmapSize, uint32_t options, uint64_t callbackID)
index c31fc24..b81a4a4 100644 (file)
@@ -103,9 +103,6 @@ private:
 
     void applyTransientZoomToLayers(double scale, WebCore::FloatPoint origin);
 
-    virtual WebCore::TransformationMatrix rootLayerTransform() const override {  return m_transform; }
-    virtual void setRootLayerTransform(const WebCore::TransformationMatrix&) override;
-
     void updateLayerHostingContext();
 
     void setRootCompositingLayer(CALayer *);
index 94f4790..5ed3ea9 100644 (file)
@@ -691,12 +691,6 @@ void TiledCoreAnimationDrawingArea::applyTransientZoomToPage(double scale, Float
     flushLayers();
 }
 
-void TiledCoreAnimationDrawingArea::setRootLayerTransform(const TransformationMatrix& transform)
-{
-    m_transform = transform;
-    [m_rootLayer setSublayerTransform:transform];
-}
-
 } // namespace WebKit
 
 #endif // !PLATFORM(IOS)