When a standard main frame load is committed, we reset the WebPage's pageScaleFactor, if it's not the default. However, if the previous page had a full-main-frame plugin that was handling page scaling itself, WebPageProxy can have a cached pageScaleFactor != 1, while WebPage now (since the plugin that was overriding (set)pageScaleFactor is gone) has a pageScaleFactor of 1, causing us to skip the call to reset the WebPage's pageScaleFactor. We need to notify WebPageProxy of the change in this case.
This causes pinch-to-zooming in PDFPlugin to break initial pinch-to-zooming in the next page loaded.
<rdar://problem/12752467>
Created attachment 176077 [details] patch
Comment on attachment 176077 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=176077&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:471 > + float pageScaleFactor = webPage->pageScaleFactor(); Don't need this temporary, and it should be a double anyway.
Comment on attachment 176077 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=176077&action=review Can you make a test case? I’m OK with this change, but it seems to be an indirect way to solve the problem, and I suspect there’s a better way. > Source/WebKit2/ChangeLog:19 > + instead of reaching into WebCore immediately. immediately -> directly > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:479 > + webPage->send(Messages::WebPageProxy::PageScaleFactorDidChange(1)); Is it really appropriate to send this message after every single load? These redundant messages don’t seem to be conveying information. Can’t the other process determine this without having to receive a cross-process message? There must be a better way to solve this. It seems bogus to just send extra scale factor did change messages when the scale factor did not change.
Hmm, you're right, there's probably somewhere that the UIProcess can do this itself already. A test case might be tricky, but I'll try (I'd like to get more of this PDFPlugin stuff tested, but that's difficult for obvious reasons).
Created attachment 176446 [details] patch How about this instead?
I don't think there's any way to test this at the moment (without building a plugin that takes over page scale that can be built on all platforms, etc. etc.).
Comment on attachment 176077 [details] patch Cleared Darin Adler's review+ from obsolete attachment 176077 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 176446 [details] patch Seems a bit inelegant to send the load type across, but I don’t have an immediate better idea and this is internal to the code so it doesn’t affect API or anything like that.
http://trac.webkit.org/changeset/139341