WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
115029
[BlackBerry] Simplify BackForwardListBlackBerry::clear
https://bugs.webkit.org/show_bug.cgi?id=115029
Summary
[BlackBerry] Simplify BackForwardListBlackBerry::clear
Xan Lopez
Reported
2013-04-23 04:23:19 PDT
The current code manually removes every single entry in the list, which in turn makes the implementation search them one by one to remove from the list. Since we want to remove all of them just set the capacity to zero, which does the same thing but more efficiently.
Attachments
simplify-clear-method.diff
(2.42 KB, patch)
2013-04-23 04:25 PDT
,
Xan Lopez
xan.lopez
: review-
Details
Formatted Diff
Diff
bflistclear.diff
(2.50 KB, patch)
2013-04-23 08:42 PDT
,
Xan Lopez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Xan Lopez
Comment 1
2013-04-23 04:25:40 PDT
Created
attachment 199212
[details]
simplify-clear-method.diff
Carlos Garcia Campos
Comment 2
2013-04-23 05:11:02 PDT
Comment on
attachment 199212
[details]
simplify-clear-method.diff View in context:
https://bugs.webkit.org/attachment.cgi?id=199212&action=review
> Source/WebKit/blackberry/WebCoreSupport/BackForwardListBlackBerry.cpp:106 > - while (!m_impl->entries().isEmpty()) > - m_impl->removeItem(m_impl->entries().last().get()); > + m_impl->setCapacity(0);
I'm not sure it's the same, removeItem doesn't change the capacity and setCapacity also removes the items from the page cache.
Xan Lopez
Comment 3
2013-04-23 05:17:46 PDT
(In reply to
comment #2
)
> > Source/WebKit/blackberry/WebCoreSupport/BackForwardListBlackBerry.cpp:106 > > - while (!m_impl->entries().isEmpty()) > > - m_impl->removeItem(m_impl->entries().last().get()); > > + m_impl->setCapacity(0); > > I'm not sure it's the same, removeItem doesn't change the capacity and setCapacity also removes the items from the page cache.
About the capacity: you are right, the patch is even wrong. What is needed is set it to zero to clear it up and then re-set to the previous capacity (so in the end it won't be changed). Not sure about the page cache, we can look into it, but fwiw other ports clear the bf list in a similar way.
Xan Lopez
Comment 4
2013-04-23 05:18:10 PDT
Comment on
attachment 199212
[details]
simplify-clear-method.diff This needs at least to not change the capacity, so marking r- for now.
Xan Lopez
Comment 5
2013-04-23 08:42:29 PDT
Created
attachment 199245
[details]
bflistclear.diff This would be correct as far as the capacity is concerned, and is what other ports do. The tests also work with it.
WebKit Commit Bot
Comment 6
2013-04-23 10:06:59 PDT
The commit-queue encountered the following flaky tests while processing
attachment 199245
[details]
: platform/mac/editing/deleting/deletionUI-single-instance.html
bug 114181
(author:
rniwa@webkit.org
) transitions/color-transition-rounding.html
bug 114182
(author:
simon.fraser@apple.com
) transitions/cubic-bezier-overflow-svg-length.html
bug 114183
(author:
peter@chromium.org
) transitions/interrupt-zero-duration.html
bug 114184
(authors:
cmarrin@apple.com
,
rniwa@webkit.org
, and
simon.fraser@apple.com
) transitions/multiple-background-transitions.html
bug 114185
(author:
simon.fraser@apple.com
) transitions/cubic-bezier-overflow-color.html
bug 114186
(author:
peter@chromium.org
) transitions/multiple-shadow-transitions.html
bug 114187
(author:
simon.fraser@apple.com
) transitions/mismatched-shadow-transitions.html
bug 114188
(author:
simon.fraser@apple.com
) transitions/color-transition-all.html
bug 114189
(authors:
ossy@webkit.org
and
simon.fraser@apple.com
) transitions/negative-delay.html
bug 114190
(author:
simon.fraser@apple.com
) transitions/cubic-bezier-overflow-shadow.html
bug 114191
(author:
peter@chromium.org
) transitions/min-max-width-height-transitions.html
bug 114192
(author:
simon.fraser@apple.com
) transitions/cancel-transition.html
bug 114193
(authors:
ojan@chromium.org
,
rniwa@webkit.org
, and
simon.fraser@apple.com
) transitions/border-radius-transition.html
bug 114194
(author:
simon.fraser@apple.com
) transitions/flex-transitions.html
bug 114195
(author:
tony@chromium.org
) transitions/mixed-type.html
bug 114196
(author:
mikelawther@chromium.org
) transitions/multiple-mask-transitions.html
bug 114197
(author:
simon.fraser@apple.com
) transitions/color-transition-premultiplied.html
bug 114198
(author:
simon.fraser@apple.com
) transitions/mismatched-shadow-styles.html
bug 114199
(author:
simon.fraser@apple.com
) transitions/mask-transitions.html
bug 114200
(authors:
ojan@chromium.org
,
oliver@apple.com
, and
simon.fraser@apple.com
) transitions/cubic-bezier-overflow-length.html
bug 114201
(author:
peter@chromium.org
) transitions/multiple-background-size-transitions.html
bug 114202
(authors:
mitz@webkit.org
and
simon.fraser@apple.com
) transitions/clip-transition.html
bug 114203
(authors:
dglazkov@chromium.org
and
simon.fraser@apple.com
) transitions/cubic-bezier-overflow-transform.html
bug 114204
(author:
peter@chromium.org
) transitions/shorthand-border-transitions.html
bug 114205
(authors:
ojan@chromium.org
and
simon.fraser@apple.com
) transitions/interrupted-accelerated-transition.html
bug 56242
(authors:
rniwa@webkit.org
,
simon.fraser@apple.com
, and
tonyg@chromium.org
) transitions/background-transitions.html
bug 114206
(author:
simon.fraser@apple.com
) http/tests/security/cookies/third-party-cookie-blocking-user-action.html
bug 114511
(authors:
ap@webkit.org
,
jochen@chromium.org
, and
rniwa@webkit.org
) http/tests/security/mixedContent/redirect-https-to-http-iframe-in-main-frame.html
bug 114208
(authors:
abarth@webkit.org
and
rniwa@webkit.org
) fast/loader/javascript-url-in-object.html
bug 114210
(authors:
rniwa@webkit.org
and
sam@webkit.org
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 7
2013-04-23 10:07:48 PDT
Comment on
attachment 199245
[details]
bflistclear.diff Clearing flags on attachment: 199245 Committed
r148972
: <
http://trac.webkit.org/changeset/148972
>
WebKit Commit Bot
Comment 8
2013-04-23 10:07:52 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.
Top of Page
Format For Printing
XML
Clone This Bug