Bug 100224 – [Qt] Detach WebContext's clients from QtWebContext

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

[Jocelyn Turcotte]

[Qt] Detach WebContext's clients from QtWebContext

[Jocelyn Turcotte]

Created attachment 170363 [details]
Patch

[Jocelyn Turcotte]

*** Bug 95968 has been marked as a duplicate of this bug. ***

[Csaba Osztrogonác]

I don't understand why we need new bug report for it ...
At least I had to add the dependency again :)

[Simon Hausmann]

Comment on attachment 170363 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=170363&action=review

> Source/WebKit2/UIProcess/API/qt/qwebiconimageprovider.cpp:53
> +    url.setPath(QLatin1Char('/') + QString::number(WTF::StringHash::hash(iconURL)));

I have the guts feeling that we might change the format of this in the future, perhaps re-introduce a context id?

This makes me wonder if we should introduce a version in the URL, like versions in REST APIs.

[Jocelyn Turcotte]

(In reply to comment #4)
> (From update of attachment 170363 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=170363&action=review
> 
> > Source/WebKit2/UIProcess/API/qt/qwebiconimageprovider.cpp:53
> > +    url.setPath(QLatin1Char('/') + QString::number(WTF::StringHash::hash(iconURL)));
> 
> I have the guts feeling that we might change the format of this in the future, perhaps re-introduce a context id?
> 
> This makes me wonder if we should introduce a version in the URL, like versions in REST APIs.

If possible it would be nice to prevent/discourage storing the image: URLs and and let them re-query them using http URLs through QWebIconImageProvider::iconURLForPageURLInContext.
Especially if they contain temporary IDs.

[Csaba Osztrogonác]

CQ won't work if the patch isn't applialble ...

[Jocelyn Turcotte]

(In reply to comment #6)
> CQ won't work if the patch isn't applialble ...

It should be appliable since its dependent patch just landed now, hopefully no other conflict has been introduced meanwhile.

[Jocelyn Turcotte]

(In reply to comment #7)
> (In reply to comment #6)
> > CQ won't work if the patch isn't applialble ...
> 
> It should be appliable since its dependent patch just landed now, hopefully no other conflict has been introduced meanwhile.

Eww ok it still doesn't work.

[Csaba Osztrogonác]

It would be great if you can add the dependcy to the depends on list of the bug.
(And next time add r? after the dependencies landed to let the EWS bots test the patch.)

[WebKit Review Bot]

Comment on attachment 170363 [details]
Patch

Clearing flags on attachment: 170363

Committed r132467: <http://trac.webkit.org/changeset/132467>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.