Bug 115029 - [BlackBerry] Simplify BackForwardListBlackBerry::clear
Summary: [BlackBerry] Simplify BackForwardListBlackBerry::clear
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-23 04:23 PDT by Xan Lopez
Modified: 2013-04-23 10:07 PDT (History)
4 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Xan Lopez 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.
Comment 1 Xan Lopez 2013-04-23 04:25:40 PDT
Created attachment 199212 [details]
simplify-clear-method.diff
Comment 2 Carlos Garcia Campos 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.
Comment 3 Xan Lopez 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.
Comment 4 Xan Lopez 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.
Comment 5 Xan Lopez 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.
Comment 6 WebKit Commit Bot 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.
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2013-04-23 10:07:52 PDT
All reviewed patches have been landed.  Closing bug.