instead of having multiple properties such as QVariant contentsScale() const; QVariant devicePixelRatio() const; QVariant initialScale() const; QVariant isScalable() const; QVariant layoutSize() const; QVariant maximumScale() const; QVariant minimumScale() const; use experimental.test.viewport and expose it as an QJsonObject (list of key-value pairs) and consolidate experimental API as follows: experimental.test.contentsScale experimental.test.viewport experimental.devicePixelRatio (as the result of removing target-densitydpi from viewport meta)
I'll take this one if you don't mind, Zalan.
Created attachment 160181 [details] Patch
Comment on attachment 160181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160181&action=review LGTM, just a few comments > Source/WebKit2/UIProcess/API/qt/qwebkittest.cpp:140 > +static QJsonObject qSizeFToJsonObject(const QSizeF& sizeF) inline maybe? > Source/WebKit2/UIProcess/qt/QtViewportHandler.cpp:237 > + float devicePixelRatio = m_devicePixelRatio; > + if (!qFuzzyCompare(devicePixelRatio, m_rawAttributes.devicePixelRatio)) { Why not compare to m_devicePixelRatio directly instead of creating a temporary variable? > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:87 > - text: "Initial scale: " + formatScale(test.initialScale) > + text: "Initial scale: " + formatScale(test.viewport.initialScale) I think having m_rawAttributes.initialScale in the info item makes not much sense since it is almost always -1 as soon as it is applied in the viewport handler. We could either remove it from the info item or change the way it is marked as used when applied.
(In reply to comment #3) > (From update of attachment 160181 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=160181&action=review > > LGTM, just a few comments > > > Source/WebKit2/UIProcess/API/qt/qwebkittest.cpp:140 > > +static QJsonObject qSizeFToJsonObject(const QSizeF& sizeF) > > inline maybe? Good idea :). > > > Source/WebKit2/UIProcess/qt/QtViewportHandler.cpp:237 > > + float devicePixelRatio = m_devicePixelRatio; > > + if (!qFuzzyCompare(devicePixelRatio, m_rawAttributes.devicePixelRatio)) { > > Why not compare to m_devicePixelRatio directly instead of creating a temporary variable? Basically, I was following a pattern that was used further down because the two data types are different (float and qreal), but realized that that place calls a method to store in the temporary variable. I'll replace it with a cast. > > > Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:87 > > - text: "Initial scale: " + formatScale(test.initialScale) > > + text: "Initial scale: " + formatScale(test.viewport.initialScale) > > I think having m_rawAttributes.initialScale in the info item makes not much sense since it is almost always -1 as soon as it is applied in the viewport handler. > We could either remove it from the info item or change the way it is marked as used when applied. True. I have no strong opinion on whether it should stay or not, maybe one of the others on cc (Kenneth? :)) has an opinion on this?
Created attachment 160365 [details] Patch
Comment on attachment 160365 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160365&action=review > Source/WebKit2/UIProcess/API/qt/qwebkittest.cpp:140 > +static inline QJsonObject qSizeFToJsonObject(const QSizeF& sizeF) you could just call it toJSonObject() as you might have more of these in the future. toJsonObject(QSizeF()) is pretty clear
Created attachment 160372 [details] Patch
Comment on attachment 160372 [details] Patch Rejecting attachment 160372 [details] from commit-queue. michaelbruening@gmail.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Comment on attachment 160372 [details] Patch Clearing flags on attachment: 160372 Committed r126570: <http://trac.webkit.org/changeset/126570>
All reviewed patches have been landed. Closing bug.