Bug 71559

Summary: [Qt] Setting QWebPreferences affect multiple WebViews
Product: WebKit Reporter: Caio Marcelo de Oliveira Filho <cmarcelo>
Component: WebKit QtAssignee: Jesus Sanchez-Palencia <jesus>
Status: RESOLVED FIXED    
Severity: Normal CC: jesus, kenneth, webkit.review.bot, zoltan
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 74403    
Attachments:
Description Flags
Example where autoloadimages preference is set in one webview but applies to the other too
none
Patch
none
Patch
none
Patch none

Caio Marcelo de Oliveira Filho
Reported 2011-11-04 07:39:42 PDT
Created attachment 113653 [details] Example where autoloadimages preference is set in one webview but applies to the other too The QWebPreferences are per WebView, but the actual implementation in WebKit2 port make them per page group and currently we only use one page group for all the views.
Attachments
Example where autoloadimages preference is set in one webview but applies to the other too (426 bytes, text/plain)
2011-11-04 07:39 PDT, Caio Marcelo de Oliveira Filho 		no flags
Details
Patch (4.14 KB, patch)
2011-12-13 10:49 PST, Jesus Sanchez-Palencia 		no flags
DetailsFormatted DiffDiff
Patch (7.98 KB, patch)
2011-12-19 12:55 PST, Jesus Sanchez-Palencia 		no flags
DetailsFormatted DiffDiff
Patch (7.98 KB, patch)
2011-12-19 13:47 PST, Jesus Sanchez-Palencia 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Jesus Sanchez-Palencia
Comment 1 2011-12-13 05:52:46 PST
Perhaps what we need here is to have one pagegroup per view.
Jesus Sanchez-Palencia
Comment 2 2011-12-13 10:49:53 PST
Created attachment 119039 [details] Patch
Alexis Menard (darktears)
Comment 3 2011-12-13 13:09:20 PST
Comment on attachment 119039 [details] Patch At least without exposing the concept of page group and context in the API(that we haven't yet figure out, and most probably won't before 5.0) it seems to me that it is the most sensible approach to take. Hopefully there is no dragon here.
Kenneth Rohde Christiansen
Comment 4 2011-12-14 00:26:21 PST
(In reply to comment #3) > (From update of attachment 119039 [details]) > At least without exposing the concept of page group and context in the API(that we haven't yet figure out, and most probably won't before 5.0) it seems to me that it is the most sensible approach to take. Hopefully there is no dragon here. I think we need to actually do that as well as security origin
Jesus Sanchez-Palencia
Comment 5 2011-12-14 08:20:13 PST
(In reply to comment #4) > I think we need to actually do that as well as security origin What do you mean, Kenneth?
Simon Hausmann
Comment 6 2011-12-19 06:42:50 PST
Comment on attachment 119039 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119039&action=review Looks good, but needs a rebase :) > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:69 > + if (!pageGroupRef) > + pageGroupRef = toAPI(WebPageGroup::create().leakRef()); I think that's okay, but this patch is going to need a rebase, and that will make it nicer :) . I suspect that after a rebase the code will look like this: RefPtr<WebPageGroup> pageGroup; if (pageGroupRef) pageGroup = toImpl(pageGroupRef); else pageGroup = WebPageGroup::create(); ... foo(pageGroup.get());
Jesus Sanchez-Palencia
Comment 7 2011-12-19 12:55:39 PST
Created attachment 119906 [details] Patch
Kenneth Rohde Christiansen
Comment 8 2011-12-19 13:20:22 PST
Comment on attachment 119906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119906&action=review > Source/WebKit2/ChangeLog:3 > + [Qt] QWebPreferences are leaking to other WebViews leaking sounds like a mem leak. I would really like you do use different wording. > Source/WebKit2/UIProcess/API/qt/tests/qmltests/WebView/tst_preferences.qml:79 > + function test_preferencesNotLeakingBetweenViews() { I wouldnt really call that leaking. That is not the definition that people are used to. test_preferencesAffectsCurrentViewOnly()
Jesus Sanchez-Palencia
Comment 9 2011-12-19 13:47:58 PST
Created attachment 119914 [details] Patch
WebKit Review Bot
Comment 10 2011-12-19 15:40:00 PST
Comment on attachment 119914 [details] Patch Clearing flags on attachment: 119914 Committed r103275: <http://trac.webkit.org/changeset/103275>
WebKit Review Bot
Comment 11 2011-12-19 15:40:04 PST
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.