Bug 56678 – Add API - QWebUserEnvironment

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Robert Hogan]

Add API - QWebUserEnvironment

[Robert Hogan]

Created attachment 86207 [details]
Patch

[Robert Hogan]

Still quite raw obviously. It depends on the patches in 56274 and 56482. Ultimately CSS font detection will need to be delegated to this class (or something like it) as well.

See https://trac.webkit.org/wiki/Fingerprinting for what I'm trying to address here.

[Robert Hogan]

Created attachment 86210 [details]
Patch

[Robert Hogan]

At first glance this class seems like a bit of a rattle-bag, and I initially intended to find a home in QWebFrame, QWebPage and co. for each of the properties delegated here. But in the end it seemed more developer-friendly to put everything in one place. 

The documentation in the class needs to be clearer that it is not a silver bullet and there needs to be more of the Fingerprinting wiki page in there. I'm also not totally comfortable with leaving Screen and Navigator entirely to QWebFrame::addJavascriptToWindowObject(). It might be better to put all privacy-relevant properties in one place. Also, the class needs a better name.

So I would like feedback on what I'm trying to do, not only here but on 56274 and 56482, which this depends on.

[Robert Hogan]

Created attachment 86218 [details]
Patch

[WebKit Review Bot]

Attachment 86218 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/qt/Api/headers.pri', u'Sourc..." exit_code: 1

Source/WebKit/qt/Api/qwebuserenvironment.h:21:  #ifndef header guard has wrong style, please use: WTF_qwebuserenvironment_h  [build/header_guard] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Benjamin Poulain]

I strongly advice starting a discussion on the mailing list to introduce any important change of API.

APIs are our tools to provide features, but it is at the same time our biggest curse because the smallest error tend to cost us a lot in maintenance, support, and can prevent us from future improvement.

Contributors who do not deal with the downside of mistake in APIs are generally not aware of that.

In this case, I understand what you are trying to do. I want to help you do that since you are doing a good job on the webkit project. But at the same time, I do not think this API is even close to something that can be integrated in a released version of QtWebKit.

First, I think you should explore the possibility of a minimal private API for your project. Since you work both on WebKit.org, and the browser, you have the full flexibility to change the browser to match the implementation.

(Please also note that the timing is pretty bad since we are stabilizing WebKit trunk with the objective of forking for QtWebKit 2.2. The stabilization will surely give less visibility to this particular work)

[Robert Hogan]

(In reply to comment #7)
> 
> First, I think you should explore the possibility of a minimal private API for your project. Since you work both on WebKit.org, and the browser, you have the full flexibility to change the browser to match the implementation.

That's a really good idea and particularly useful here as the best way of exposing the information browsers need to deal with fingerprinting is likely to develop over time. A private API in DumpRenderTreeSupportQt or similar is exactly the sort of place to get a better handle on the final API requirements.

Do you have any concerns about the changes in 56274 and 56482?

[Kenneth Rohde Christiansen]

(In reply to comment #7)
> I strongly advice starting a discussion on the mailing list to introduce any important change of API.
> 
> APIs are our tools to provide features, but it is at the same time our biggest curse because the smallest error tend to cost us a lot in maintenance, support, and can prevent us from future improvement.
> 
> Contributors who do not deal with the downside of mistake in APIs are generally not aware of that.
> 
> In this case, I understand what you are trying to do. I want to help you do that since you are doing a good job on the webkit project. But at the same time, I do not think this API is even close to something that can be integrated in a released version of QtWebKit.
> 
> First, I think you should explore the possibility of a minimal private API for your project. Since you work both on WebKit.org, and the browser, you have the full flexibility to change the browser to match the implementation.
> 
> (Please also note that the timing is pretty bad since we are stabilizing WebKit trunk with the objective of forking for QtWebKit 2.2. The stabilization will surely give less visibility to this particular work)

The API basically exposes the Chrome Client. Also changing most of these will result in wrongly shown contents (especially on mobile platforms), so if not treated the right way it can be a can of wormes. We need to be very careful about this.

[Robert Hogan]

(In reply to comment #9)
> 
> The API basically exposes the Chrome Client. Also changing most of these will result in wrongly shown contents (especially on mobile platforms), so if not treated the right way it can be a can of wormes. We need to be very careful about this.

You're right. The pros and cons the client needs to weigh up are outlined here:

https://trac.webkit.org/wiki/Fingerprinting#WindowObject

and in the links there. The approach of Torbutton and Torora is to restrict the size of the actual window, e.g. by multiples of px, so that the rendered content is not horribly disconnected from the actual window size. Another policy a client could use is to fix on a minimum rendered page size and use that regardless if the window is actually larger.

This is not a very pleasant trade-off to have to make and the right choice will differ from platform to platform and client to client.

[Tor Arne Vestbø]

Comment on attachment 86218 [details]
Patch

Agree with Benjamin, a bit more discussion is needed.