WebPageGroup does not need to be created in creating EwkView when the pageGroupRef is 0 because the default WebPageGroup(m_defaultPageGroup) is created in WebContext constructor. As a side effect on the duplicated initialization, pageGroupID increases even when the EwkView is created with default context. The issue can be easily detected with the following test case. (Even the window name is same, the new window is created.) http://www.w3schools.com/js/tryit.asp?filename=try_win_name
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>