View scale changes are temporarily lost after restoring a page from the page cache
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 May 2015 18:11:38 +0000 (18:11 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 May 2015 18:11:38 +0000 (18:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144934

Reviewed by Brady Eidson.

* history/CachedPage.cpp:
(WebCore::CachedPage::CachedPage):
(WebCore::CachedPage::restore):
(WebCore::CachedPage::clear):
* history/CachedPage.h:
(WebCore::CachedPage::markForDeviceOrPageScaleChanged): Renamed.
* history/PageCache.cpp:
(WebCore::PageCache::markPagesForDeviceOrPageScaleChanged): Renamed.
* history/PageCache.h:
Rename PageCache/CachedPage methods to make it more clear that they
will eventually result in calling deviceOrPageScaleFactorChanged().
Also, use modern initialization for CachedPage members.

* loader/HistoryController.cpp:
(WebCore::HistoryController::saveScrollPositionAndViewStateToItem):
(WebCore::HistoryController::restoreScrollPositionAndViewState):
Store the pageScaleFactor on HistoryItem with the view scale factored out,
because the view scale can change while the page is in the page cache, and
WebCore needs a way - without consulting with WebKit2 - to apply the changed
view scale to the cached page scale.

* page/Page.cpp:
(WebCore::Page::setViewScaleFactor):
(WebCore::Page::setDeviceScaleFactor):
* page/Page.h:
(WebCore::Page::viewScaleFactor):
Keep track of the viewScaleFactor, and mark all pages in the page cache
as needing to call deviceOrPageScaleFactorChanged and do a full style recalc
when they come back from the page cache.

For now, we expect all callers of setPageScaleFactor (including WebKit2 and
HistoryController) to multiply the viewScale in manually, to avoid the
significant amount of change in WebCore that would be required to keep them
totally separately.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::WebPage):
(WebKit::WebPage::scalePage):
(WebKit::WebPage::scalePageInViewCoordinates):
(WebKit::WebPage::pageScaleFactor):
(WebKit::WebPage::viewScaleFactor):
(WebKit::WebPage::scaleView):
* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::viewScaleFactor): Deleted.
Get rid of m_viewScaleFactor, instead using Page::viewScaleFactor.

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

Source/WebCore/ChangeLog
Source/WebCore/history/CachedPage.cpp
Source/WebCore/history/CachedPage.h
Source/WebCore/history/PageCache.cpp
Source/WebCore/history/PageCache.h
Source/WebCore/loader/HistoryController.cpp
Source/WebCore/page/Page.cpp
Source/WebCore/page/Page.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.h

index b6f1d50..c4d6cd0 100644 (file)
@@ -1,3 +1,45 @@
+2015-05-13  Timothy Horton  <timothy_horton@apple.com>
+
+        View scale changes are temporarily lost after restoring a page from the page cache
+        https://bugs.webkit.org/show_bug.cgi?id=144934
+
+        Reviewed by Brady Eidson.
+
+        * history/CachedPage.cpp:
+        (WebCore::CachedPage::CachedPage):
+        (WebCore::CachedPage::restore):
+        (WebCore::CachedPage::clear):
+        * history/CachedPage.h:
+        (WebCore::CachedPage::markForDeviceOrPageScaleChanged): Renamed.
+        * history/PageCache.cpp:
+        (WebCore::PageCache::markPagesForDeviceOrPageScaleChanged): Renamed.
+        * history/PageCache.h:
+        Rename PageCache/CachedPage methods to make it more clear that they
+        will eventually result in calling deviceOrPageScaleFactorChanged().
+        Also, use modern initialization for CachedPage members.
+
+        * loader/HistoryController.cpp:
+        (WebCore::HistoryController::saveScrollPositionAndViewStateToItem):
+        (WebCore::HistoryController::restoreScrollPositionAndViewState):
+        Store the pageScaleFactor on HistoryItem with the view scale factored out,
+        because the view scale can change while the page is in the page cache, and
+        WebCore needs a way - without consulting with WebKit2 - to apply the changed
+        view scale to the cached page scale.
+
+        * page/Page.cpp:
+        (WebCore::Page::setViewScaleFactor):
+        (WebCore::Page::setDeviceScaleFactor):
+        * page/Page.h:
+        (WebCore::Page::viewScaleFactor):
+        Keep track of the viewScaleFactor, and mark all pages in the page cache
+        as needing to call deviceOrPageScaleFactorChanged and do a full style recalc
+        when they come back from the page cache.
+
+        For now, we expect all callers of setPageScaleFactor (including WebKit2 and
+        HistoryController) to multiply the viewScale in manually, to avoid the
+        significant amount of change in WebCore that would be required to keep them
+        totally separately.
+
 2015-05-12  Zan Dobersek  <zdobersek@igalia.com>
 
         Reduce TransformationMatrix copies in MatrixTransformOperation, Matrix3DTransformOperation
index 84aa030..ef71ee6 100644 (file)
@@ -52,12 +52,6 @@ DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, cachedPageCounter, ("Cached
 CachedPage::CachedPage(Page& page)
     : m_expirationTime(monotonicallyIncreasingTime() + page.settings().backForwardCacheExpirationInterval())
     , m_cachedMainFrame(std::make_unique<CachedFrame>(page.mainFrame()))
-    , m_needStyleRecalcForVisitedLinks(false)
-    , m_needsFullStyleRecalc(false)
-#if ENABLE(VIDEO_TRACK)
-    , m_needsCaptionPreferencesChanged(false)
-#endif
-    , m_needsDeviceScaleChanged(false)
 {
 #ifndef NDEBUG
     cachedPageCounter.increment();
@@ -111,7 +105,7 @@ void CachedPage::restore(Page& page)
             frame->document()->visitedLinkState().invalidateStyleForAllLinks();
     }
 
-    if (m_needsDeviceScaleChanged)
+    if (m_needsDeviceOrPageScaleChanged)
         page.mainFrame().deviceOrPageScaleFactorChanged();
 
     if (m_needsFullStyleRecalc)
@@ -135,7 +129,7 @@ void CachedPage::clear()
 #if ENABLE(VIDEO_TRACK)
     m_needsCaptionPreferencesChanged = false;
 #endif
-    m_needsDeviceScaleChanged = false;
+    m_needsDeviceOrPageScaleChanged = false;
 }
 
 bool CachedPage::hasExpired() const
index 5636de1..9340ed2 100644 (file)
@@ -55,17 +55,17 @@ public:
     void markForCaptionPreferencesChanged() { m_needsCaptionPreferencesChanged = true; }
 #endif
 
-    void markForDeviceScaleChanged() { m_needsDeviceScaleChanged = true; }
+    void markForDeviceOrPageScaleChanged() { m_needsDeviceOrPageScaleChanged = true; }
 
 private:
     double m_expirationTime;
     std::unique_ptr<CachedFrame> m_cachedMainFrame;
-    bool m_needStyleRecalcForVisitedLinks;
-    bool m_needsFullStyleRecalc;
+    bool m_needStyleRecalcForVisitedLinks { false };
+    bool m_needsFullStyleRecalc { false };
 #if ENABLE(VIDEO_TRACK)
-    bool m_needsCaptionPreferencesChanged;
+    bool m_needsCaptionPreferencesChanged { false };
 #endif
-    bool m_needsDeviceScaleChanged;
+    bool m_needsDeviceOrPageScaleChanged { false };
 };
 
 } // namespace WebCore
index 839f376..bcbdef2 100644 (file)
@@ -390,12 +390,12 @@ void PageCache::markPagesForFullStyleRecalc(Page& page)
     }
 }
 
-void PageCache::markPagesForDeviceScaleChanged(Page& page)
+void PageCache::markPagesForDeviceOrPageScaleChanged(Page& page)
 {
     for (auto& item : m_items) {
         CachedPage& cachedPage = *item->m_cachedPage;
         if (&page.mainFrame() == &cachedPage.cachedMainFrame()->view()->frame())
-            cachedPage.markForDeviceScaleChanged();
+            cachedPage.markForDeviceOrPageScaleChanged();
     }
 }
 
index f92140f..9fab802 100644 (file)
@@ -64,7 +64,7 @@ public:
     WEBCORE_EXPORT void markPagesForVisitedLinkStyleRecalc();
     // Will mark all cached pages associated with the given page as needing style recalc.
     void markPagesForFullStyleRecalc(Page&);
-    void markPagesForDeviceScaleChanged(Page&);
+    void markPagesForDeviceOrPageScaleChanged(Page&);
 #if ENABLE(VIDEO_TRACK)
     void markPagesForCaptionPreferencesChanged();
 #endif
index 76a0964..ae7db3d 100644 (file)
@@ -86,7 +86,7 @@ void HistoryController::saveScrollPositionAndViewStateToItem(HistoryItem* item)
 
     Page* page = m_frame.page();
     if (page && m_frame.isMainFrame())
-        item->setPageScaleFactor(page->pageScaleFactor());
+        item->setPageScaleFactor(page->pageScaleFactor() / page->viewScaleFactor());
 
     // FIXME: It would be great to work out a way to put this code in WebCore instead of calling through to the client.
     m_frame.loader().client().saveViewStateToItem(item);
@@ -151,7 +151,7 @@ void HistoryController::restoreScrollPositionAndViewState()
     if (view && !view->wasScrolledByUser()) {
         Page* page = m_frame.page();
         if (page && m_frame.isMainFrame() && m_currentItem->pageScaleFactor())
-            page->setPageScaleFactor(m_currentItem->pageScaleFactor(), m_currentItem->scrollPoint());
+            page->setPageScaleFactor(m_currentItem->pageScaleFactor() * page->viewScaleFactor(), m_currentItem->scrollPoint());
         else
             view->setScrollPosition(m_currentItem->scrollPoint());
     }
index 14d92d9..16476a3 100644 (file)
@@ -832,6 +832,16 @@ void Page::setPageScaleFactor(float scale, const IntPoint& origin, bool inStable
 #endif
 }
 
+void Page::setViewScaleFactor(float scale)
+{
+    if (m_viewScaleFactor == scale)
+        return;
+
+    m_viewScaleFactor = scale;
+    PageCache::singleton().markPagesForDeviceOrPageScaleChanged(*this);
+    PageCache::singleton().markPagesForFullStyleRecalc(*this);
+}
+
 void Page::setDeviceScaleFactor(float scaleFactor)
 {
     ASSERT(scaleFactor > 0);
@@ -845,7 +855,7 @@ void Page::setDeviceScaleFactor(float scaleFactor)
     setNeedsRecalcStyleInAllFrames();
 
     mainFrame().deviceOrPageScaleFactorChanged();
-    PageCache::singleton().markPagesForDeviceScaleChanged(*this);
+    PageCache::singleton().markPagesForDeviceOrPageScaleChanged(*this);
 
     PageCache::singleton().markPagesForFullStyleRecalc(*this);
     GraphicsContext::updateDocumentMarkerResources();
index d030b4b..cfbef66 100644 (file)
@@ -260,6 +260,11 @@ public:
     WEBCORE_EXPORT void setPageScaleFactor(float scale, const IntPoint& origin, bool inStableState = true);
     float pageScaleFactor() const { return m_pageScaleFactor; }
 
+    // The view scale factor is multiplied into the page scale factor by all
+    // callers of setPageScaleFactor.
+    WEBCORE_EXPORT void setViewScaleFactor(float);
+    float viewScaleFactor() const { return m_viewScaleFactor; }
+
     WEBCORE_EXPORT void setZoomedOutPageScaleFactor(float);
     float zoomedOutPageScaleFactor() const { return m_zoomedOutPageScaleFactor; }
 
@@ -520,6 +525,7 @@ private:
     float m_pageScaleFactor;
     float m_zoomedOutPageScaleFactor;
     float m_deviceScaleFactor;
+    float m_viewScaleFactor { 1 };
 
     float m_topContentInset;
     
index 857040b..3af36a5 100644 (file)
@@ -1,3 +1,21 @@
+2015-05-13  Timothy Horton  <timothy_horton@apple.com>
+
+        View scale changes are temporarily lost after restoring a page from the page cache
+        https://bugs.webkit.org/show_bug.cgi?id=144934
+
+        Reviewed by Brady Eidson.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::WebPage):
+        (WebKit::WebPage::scalePage):
+        (WebKit::WebPage::scalePageInViewCoordinates):
+        (WebKit::WebPage::pageScaleFactor):
+        (WebKit::WebPage::viewScaleFactor):
+        (WebKit::WebPage::scaleView):
+        * WebProcess/WebPage/WebPage.h:
+        (WebKit::WebPage::viewScaleFactor): Deleted.
+        Get rid of m_viewScaleFactor, instead using Page::viewScaleFactor.
+
 2015-05-13  Sungmann Cho  <sungmann.cho@navercorp.com>
 
         Minor cleanups to PluginProxy.cpp.
index cdce75a..742e3eb 100644 (file)
@@ -338,7 +338,6 @@ WebPage::WebPage(uint64_t pageID, const WebPageCreationParameters& parameters)
     , m_processSuppressionEnabled(true)
     , m_userActivity("Process suppression disabled for page.")
     , m_pendingNavigationID(0)
-    , m_viewScaleFactor(parameters.viewScaleFactor)
 #if ENABLE(WEBGL)
     , m_systemWebGLPolicy(WebGLAllowCreation)
 #endif
@@ -506,8 +505,8 @@ WebPage::WebPage(uint64_t pageID, const WebPageCreationParameters& parameters)
 #endif
     m_page->settings().setAppleMailPaginationQuirkEnabled(parameters.appleMailPaginationQuirkEnabled);
 
-    if (m_viewScaleFactor != 1)
-        scalePage(1, IntPoint());
+    if (parameters.viewScaleFactor != 1)
+        scaleView(parameters.viewScaleFactor);
 }
 
 void WebPage::reinitializeWebPage(const WebPageCreationParameters& parameters)
@@ -1374,7 +1373,7 @@ void WebPage::windowScreenDidChange(uint32_t displayID)
 
 void WebPage::scalePage(double scale, const IntPoint& origin)
 {
-    double totalScale = scale * m_viewScaleFactor;
+    double totalScale = scale * viewScaleFactor();
     bool willChangeScaleFactor = totalScale != totalScaleFactor();
 
 #if PLATFORM(IOS)
@@ -1407,7 +1406,7 @@ void WebPage::scalePage(double scale, const IntPoint& origin)
 
 void WebPage::scalePageInViewCoordinates(double scale, IntPoint centerInViewCoordinates)
 {
-    double totalScale = scale * m_viewScaleFactor;
+    double totalScale = scale * viewScaleFactor();
     if (totalScale == totalScaleFactor())
         return;
 
@@ -1428,26 +1427,32 @@ double WebPage::totalScaleFactor() const
 
 double WebPage::pageScaleFactor() const
 {
-    return totalScaleFactor() / m_viewScaleFactor;
+    return totalScaleFactor() / viewScaleFactor();
+}
+
+double WebPage::viewScaleFactor() const
+{
+    return m_page->viewScaleFactor();
 }
 
 void WebPage::scaleView(double scale)
 {
-    float pageScale = pageScaleFactor();
+    if (viewScaleFactor() == scale)
+        return;
 
-    double scaleRatio = scale / m_viewScaleFactor;
+    float pageScale = pageScaleFactor();
 
     IntPoint scrollPositionAtNewScale;
     if (FrameView* mainFrameView = m_page->mainFrame().view()) {
+        double scaleRatio = scale / viewScaleFactor();
         scrollPositionAtNewScale = mainFrameView->scrollPosition();
         scrollPositionAtNewScale.scale(scaleRatio, scaleRatio);
     }
 
-    m_viewScaleFactor = scale;
+    m_page->setViewScaleFactor(scale);
     scalePage(pageScale, scrollPositionAtNewScale);
 }
 
-
 #if PLATFORM(COCOA)
 void WebPage::scaleViewAndUpdateGeometryFenced(double scale, IntSize viewSize, uint64_t callbackID)
 {
index b25fde0..240572f 100644 (file)
@@ -359,7 +359,7 @@ public:
     void scalePageInViewCoordinates(double scale, WebCore::IntPoint centerInViewCoordinates);
     double pageScaleFactor() const;
     double totalScaleFactor() const;
-    double viewScaleFactor() const { return m_viewScaleFactor; }
+    double viewScaleFactor() const;
     void scaleView(double scale);
 #if PLATFORM(COCOA)
     void scaleViewAndUpdateGeometryFenced(double scale, WebCore::IntSize viewSize, uint64_t callbackID);
@@ -1359,8 +1359,6 @@ private:
 
     uint64_t m_pendingNavigationID;
 
-    double m_viewScaleFactor { 1 };
-
 #if ENABLE(WEBGL)
     WebCore::WebGLLoadPolicy m_systemWebGLPolicy;
 #endif