We need plumbing to route the scaling factor from GTK+ to WebCore.
Created attachment 229165 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Owen, I made a few stylistic changes and moved the attachment of the property notify handler to webkitWebViewBaseCreateWebPage. This allowed me to transform the check for pageProxy into an assertion. Do you mind taking a quick look at the patch to see if I inadvertently introduced any non-cosmetic changes? :)
As far as can tell from comparing the patches, the changes seem to be functionally equivalent.
Okay. Thanks. I'll land this.
Comment on attachment 229165 [details] Patch Clearing flags on attachment: 229165 Committed r167168: <http://trac.webkit.org/changeset/167168>
All reviewed patches have been landed. Closing bug.
Comment on attachment 229165 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229165&action=review > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:973 > + ASSERT(priv->pageProxy); This shouldn't happen, the signal is connected after creating the page and the page proxy is destroyed on finalize when the private struct is destroyed. If we want to be extra sure, we can save the signal handler and disconnect the signal on dispose since the page is closed there, any further call to page proxy after closed will be simply ignored. > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:996 > + g_signal_connect(object, "notify::scale-factor", G_CALLBACK(scaleFactorChanged), NULL); What is object here? does this even build? Use nullptr, instead of NULL.
(In reply to comment #8) > (From update of attachment 229165 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=229165&action=review > > > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:973 > > + ASSERT(priv->pageProxy); > > This shouldn't happen, the signal is connected after creating the page and the page proxy is destroyed on finalize when the private struct is destroyed. If we want to be extra sure, we can save the signal handler and disconnect the signal on dispose since the page is closed there, any further call to page proxy after closed will be simply ignored. "This shouldn't happen" is precisely the situation to use an assertion in! > > > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:996 > > + g_signal_connect(object, "notify::scale-factor", G_CALLBACK(scaleFactorChanged), NULL); > > What is object here? does this even build? Use nullptr, instead of NULL. Ah, I let this one slip through. I can land a fix for it.
Comment on attachment 229165 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229165&action=review > Source/WebCore/platform/graphics/cairo/CairoUtilities.cpp:270 > + ASSERT_UNUSED(xScale, 1 == xScale); Has this been tested in Debug builds? This ASSERT hits in the Debug bot, see http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r167177%20(41643)/results.html
Re-opened since this is blocked by bug 131589
(In reply to comment #10) > (From update of attachment 229165 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=229165&action=review > > > Source/WebCore/platform/graphics/cairo/CairoUtilities.cpp:270 > > + ASSERT_UNUSED(xScale, 1 == xScale); > > Has this been tested in Debug builds? This ASSERT hits in the Debug bot, see http://build.webkit.org/results/GTK%20Linux%2064-bit%20Debug/r167177%20(41643)/results.html The problem is that the tests can also set the device scale factor using WKPageSetCustomBackingScaleFactor, even if we don't set the intrinsic device scale factor to the web page proxy. So we might end up with a scaleFactor > 1 at any moment. We could make sure the custom scale factor is always 1 when hidpi is not supported for GTK+. I'll rebase the patch.
Created attachment 232244 [details] Rebased patch
Comment on attachment 232244 [details] Rebased patch View in context: https://bugs.webkit.org/attachment.cgi?id=232244&action=review > Source/WebKit2/UIProcess/WebPageProxy.cpp:1853 > + return; Like we discussed, please add a FIXME saying that this is only temporary.
Committed r169445: <http://trac.webkit.org/changeset/169445>