Bug 100224

Summary: [Qt] Detach WebContext's clients from QtWebContext
Product: WebKit Reporter: Jocelyn Turcotte <jturcotte>
Component: New BugsAssignee: Jocelyn Turcotte <jturcotte>
Status: RESOLVED FIXED    
Severity: Critical CC: hausmann, jturcotte, ossy, webkit.review.bot
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 70236, 76773    
Attachments:
Description Flags
Patch none

Jocelyn Turcotte
Reported 2012-10-24 03:46:22 PDT
[Qt] Detach WebContext's clients from QtWebContext
Attachments
Patch (15.25 KB, patch)
2012-10-24 04:41 PDT, Jocelyn Turcotte 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jocelyn Turcotte
Comment 1 2012-10-24 04:41:20 PDT
Created attachment 170363 [details] Patch
Jocelyn Turcotte
Comment 2 2012-10-24 07:01:20 PDT
*** Bug 95968 has been marked as a duplicate of this bug. ***
Csaba Osztrogonác
Comment 3 2012-10-24 07:05:49 PDT
I don't understand why we need new bug report for it ... At least I had to add the dependency again :)
Simon Hausmann
Comment 4 2012-10-24 11:37:42 PDT
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
Comment 5 2012-10-25 03:44:56 PDT
(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
Comment 6 2012-10-25 03:47:33 PDT
CQ won't work if the patch isn't applialble ...
Jocelyn Turcotte
Comment 7 2012-10-25 03:54:40 PDT
(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
Comment 8 2012-10-25 03:56:33 PDT
(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
Comment 9 2012-10-25 03:58:37 PDT
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 10 2012-10-25 04:16:50 PDT
Comment on attachment 170363 [details] Patch Clearing flags on attachment: 170363 Committed r132467: <http://trac.webkit.org/changeset/132467>
WebKit Review Bot
Comment 11 2012-10-25 04:16:54 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.