User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041610 Minefield/3.0pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041610 Minefield/3.0pre Deleting large live bookmark makes Firefox unresponsive for several seconds. Reproducible: Always Steps to Reproduce: 1. Navigate to http://www.thejournal.com/the/rss/ 2. Click the button to subscribe to this feed using Live Bookmarks to the Bookmarks Toolbar. 3. Allow the feed to populate. 4. Right click the Live Bookmark, select Delete. Actual Results: The browser hangs for 45 seconds at least, accepting no input. Once the delete operation completes, the browser resumes normal functionality. Expected Results: The Live Bookmark should disappear from the bookmark toolbar quickly, and the user interface should not hang. Deleting the Live Bookmark should not take 45 seconds or longer. The user interface should not hang. If there is really some operation that needs so long to complete, it should be started and run in the background, not locking up the UI.

That looks like a _very_ inefficient SQL query. On my system it took a full 2 minutes to delete what was added in a matter of seconds.

I revisited this today with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050706 Minefield/3.0pre and noticed that there is a very large amount of disk i/o that occurs during the time Firefox is hanging, waiting for the live bookmark to delete. I am not sure exactly what is being accessed or deleted. It seems like Firefox is looking at the Live Bookmark like a directory full of files saved on the hard drive, instead of as a list of links cached someplace in memory for the current session.

I revisited this again today, this time with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081117 Minefield/3.1b2pre and noticed a new dialog that I had not before seen. The dialog appeared a total of seven times for me, each time asking whether or not to stop a script. Each time I clicked continue until the live bookmark had been deleted. The text of that dialog follows: A script on this page may be busy, or it may have stopped responding. You can stop the script now, or you can continue to see if the script will complete. Script: chrome://browser/content/places/toolbar.xml:553

easily reproducible on a clean profile, we should take a look if we can gain something on this test case for 3.1 since it could help also other delete cases.

there are 2 bottlenecks here, the former is due to removeFolder, that has to remove all childrens (and luckily this is a bit faster now, still not perfect though, but we can track this later in bug 428459). The latter and much bigger bottleneck is due to the transactionService, even if this is a livemark we save all of its children. Well i think we should not, children of a livemark are known to change, so there's no point in saving something we can reload directly from the feed. This patch adds a removeLivemarkTransaction, and with this the big livemark removal takes 1-2 seconds. On undo we recreate the livemark and reload its contents from the feed, re-registering it with livemark service (something we were not doing before, so that's another bug.) asking a first-pass review, i have to write a test for this before going on.

Comment on attachment 349564 [details] [diff] [review] wip 0.1 >+function placesRemoveLivemarkTransaction(aFolderId) { >+ this.redoTransaction = this.doTransaction; >+ this._id = aFolderId; >+ this._title = PlacesUtils.bookmarks.getItemTitle(this._id); >+ this._container = PlacesUtils.bookmarks.getFolderIdForItem(this._id); >+ var annos = PlacesUtils.getAnnotationsForItem(this._id); >+ // Exclude livemark related annotations, those will be recreated automatically >+ this._annotations = annos.filter(function(aValue, aIndex, aArray) { >+ return !aValue.name.match(/^livemark/); >+ }); please match exactly on the annotations you intend to exclude, as you might match annotations from extensions otherwise. r=me for the approach.

mh, i did that because we have a lot of different annotations there, and if we add more annotations in the service we will also have to update the transaction manager. Hwv i don't see other solutions if we don't want to force extension developers to use a different annotation root.

addes a test for this and cleaned up a bit transaction manager livemarks tests, plus addressed previous comment.

Comment on attachment 352098 [details] [diff] [review] patch v1.1 oops wrong patch

Sorry this is the correct one

Comment on attachment 352099 [details] [diff] [review] correct patch v1.0 >+ // Testing remove livemark >+ // Set an annotation an check that we don't lose it on undo nit: s/an/and/ r=me

addressed nit

pushed to trunk: http://hg.mozilla.org/mozilla-central/rev/a8b303937581

askin in litmus because maybe the big livemark posted here could be useful for manual testing

Comment on attachment 352171 [details] [diff] [review] correct patch v1.1 asking approval 1.9.1. this is making livemarks delete really faster (and general all folder deletes that contain livemarks). Low risk since come with a complete unit test.

if we try to load a not existant livemark uri we leak loadgroup and other things

Comment on attachment 352231 [details] [diff] [review] followup i'll move the issue to a new bug since i think it's a livemark service bug.

Comment on attachment 352171 [details] [diff] [review] correct patch v1.1 a191=beltzner

pushed to 1.9.1 branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/58e38927aaca

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090123 Shiretoko/3.1b3pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090123 Minefield/3.2a1pre deleting takes 2-4 seconds now.

added https://litmus.mozilla.org/show_test.cgi?id=7540 to litmus

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv