Summary: | [EFL][WK2] Duplicated WebPageGroup initialization | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jinwoo Song <jinwoo7.song> | ||||||
Component: | WebKit EFL | Assignee: | Jinwoo Song <jinwoo7.song> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, gyuyoung.kim, kangil.han, kenneth, lucas.de.marchi, rakuco, ryuan.choi, webkit.review.bot, yael | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 103604, 103692 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Jinwoo Song
2012-11-26 01:41:37 PST
Created attachment 175942 [details]
Patch
LGTM, thanks. :) LGTM too Comment on attachment 175942 [details] Patch Clearing flags on attachment: 175942 Committed r135973: <http://trac.webkit.org/changeset/135973> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 103604 ewk_setting API test cases should be fixed to land this patch. (bug 103692) Kenneth, could you please take a look at this? This patch changes the behavior a bit. All the views from the default context will be in the same PageGroup, meaning that they will share the settings. Previously, we were creating a new PageGroup for each view and therefore, each view had its own settings. Comment on attachment 175942 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175942&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:512 > ASSERT(!smartData->priv); > - RefPtr<WebPageGroup> pageGroup = pageGroupRef ? toImpl(pageGroupRef) : WebPageGroup::create(); > - smartData->priv = new EwkViewImpl(ewkView, context, pageGroup, behavior); > + smartData->priv = new EwkViewImpl(ewkView, context, toImpl(pageGroupRef), behavior); > return ewkView; I wonder whether we should add a comment? (In reply to comment #8) > Kenneth, could you please take a look at this? > > This patch changes the behavior a bit. All the views from the default context will be in the same PageGroup, meaning that they will share the settings. > Previously, we were creating a new PageGroup for each view and therefore, each view had its own settings. That behavior was wrong. If a page creates a named popup, and then tries to create it again with the same name, we were creating a new popup instead of replacing the existing one. (In reply to comment #9) > (From update of attachment 175942 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=175942&action=review > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:512 > > ASSERT(!smartData->priv); > > - RefPtr<WebPageGroup> pageGroup = pageGroupRef ? toImpl(pageGroupRef) : WebPageGroup::create(); > > - smartData->priv = new EwkViewImpl(ewkView, context, pageGroup, behavior); > > + smartData->priv = new EwkViewImpl(ewkView, context, toImpl(pageGroupRef), behavior); > > return ewkView; > > I wonder whether we should add a comment? I added the comment here. Created attachment 177086 [details]
patch
Patch with more comments.
Comment on attachment 177086 [details] patch Clearing flags on attachment: 177086 Committed r137423: <http://trac.webkit.org/changeset/137423> All reviewed patches have been landed. Closing bug. |