Steps to Reproduce (through the code!): 1. Set a non-1 viewScale. 2. Navigate. 3. Set a different non-1 viewScale. 4. Go back. Expected: WebCore's pageScaleFactor should be equal to the viewScale set in #3. The transform on the tiles should be a scale equal to the viewScale set in #3. Actual: WebCore's pageScaleFactor is equal to the viewScale set in #1. The transform on the tiles is a scale equal to the viewScale set in #1. Notes: This is happening because the pageScaleFactor stored on HistoryItem is WebCore's pageScaleFactor, which has the viewScale multiplied into it. When we come back from the PageCache, HistoryController does a setPageScaleFactor() with the stored scale (with the old viewScale multiplied into it), disregarding any viewScale changes since that item went into the PageCache. Since WebKit2 keeps pageScale and viewScale separately, doing a small pinch zoom will push the correct pageScaleFactor to WebCore and recover from this situation. To fix this, I think we're going to have to inform WebCore of the viewScale, something I had previously been trying to avoid -- but only for the purposes of the PageCache, so hopefully it will be OK.
There's a second hilarity here where, once we fix HistoryController to apply the new viewScale when restoring the pageScale, since the Page's m_pageScaleFactor is already updated to the new value, setPageScaleFactor() will bail and fail to update the scale on the tile container layer. This never mattered before because the scale that HistoryController handed to setPageScaleFactor() was always the same as the scale already applied to the layer that is coming back from the PageCache (which is no longer true). I've worked around this by notifying the frames of a pageScaleFactor change if either of the scales is now different from what was stored in the history item -- I'm not sure this is the best fix. Alternatively, we could make setPageScaleFactor take a "no really, I mean it" parameter...
... or, check with the compositor's notion of the current page scale factor and avoid bailing if it doesn't match m_pageScaleFactor. Or something.
Created attachment 253027 [details] Patch
(In reply to comment #1) > I've worked around this by notifying the frames of a pageScaleFactor change > if either of the scales is now different from what was stored in the history > item -- I'm not sure this is the best fix. It turns out there was already a mechanism to do this that I just made use of for this patch (after reinventing it :D).
Comment on attachment 253027 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253027&action=review > Source/WebCore/page/Page.cpp:839 > + m_viewScaleFactor = scale; > + PageCache::singleton().markPagesForDeviceOrPageScaleChanged(*this); > + PageCache::singleton().markPagesForFullStyleRecalc(*this); Should this early-return if the scale hasn't changed?
Comment on attachment 253027 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253027&action=review > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1451 > } What does this function exactly do? Does it scale the view scale factor: new_scale_factor = current_scale_factor * scale? Or does it set the view scale factor: new_scale_factor = scale? I think it is the later and I think this function should be renamed setViewScaleFactor()?
Comment on attachment 253027 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=253027&action=review >> Source/WebCore/page/Page.cpp:839 >> + PageCache::singleton().markPagesForFullStyleRecalc(*this); > > Should this early-return if the scale hasn't changed? Probably! The WebKit2 one should too, probably. >> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1451 >> } > > What does this function exactly do? Does it scale the view scale factor: new_scale_factor = current_scale_factor * scale? Or does it set the view scale factor: new_scale_factor = scale? I think it is the later and I think this function should be renamed setViewScaleFactor()? You're talking about scaleView()? The naming of scaleView matches the longstanding scalePage() message name, which does the same thing. Not sure why, but I'm not going to change it now.
http://trac.webkit.org/changeset/184290