Closed Bug 730013 Opened 11 years ago Closed 11 years ago

nsTransactionManager (and undo/redo stacks) are often in CC graph, even if the manager is alive


(Core :: DOM: Core & HTML, defect)

12 Branch
Not set



Tracking Status
firefox12 + fixed


(Reporter: smaug, Assigned: smaug)


(Whiteboard: [qa?])


(2 files)

Attached patch patchSplinter Review
The worst case is probably locationbar, where we may get quite large undo/redo stacks,
and lots of transactions.
It is a bit difficult to add any CanSkip methods to the manager, since the ownership
rules aren't that clear. (the manager is used also elsewhere than just with editor).
So, the patch just makes us addref/release less so that txmanager doesn't end up to
purple buffer in the most common case (this makes the code also a tiny bit faster).
The API of nsIEditor gets also a bit better, since it already has undo(cnt)/redo(cnt), but
no way to tell how many times undo/redo is effective.
Attachment #600073 - Flags: review?(ehsan)
This might be good for FF12 too, since the patch should be very safe, and it reduces
CCs times during typing to locationbar.
Comment on attachment 600073 [details] [diff] [review]

Review of attachment 600073 [details] [diff] [review]:

This is really good, and I'd be fine if you would want to backport it to branches.
Attachment #600073 - Flags: review?(ehsan) → review+
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to Olli Pettay [:smaug] from comment #2)
> This might be good for FF12 too, since the patch should be very safe, and it
> reduces
> CCs times during typing to locationbar.

We haven't seen reports of pausing in the locationbar - could this be related to other pauses as well?
Target Milestone: --- → mozilla13
Yes. The patch should help with CC runs which happen when user is typing to
location bar or to any input/textarea element.
Comment on attachment 600073 [details] [diff] [review]

[Approval Request Comment]
Regression caused by (bug #): N/A 
User impact if declined: Possibly higher CC times while typing
Testing completed (on m-c, etc.): Landed m-c 2012-02-23
Risk to taking this patch (and alternatives if risky):
  Should be very low risk patch
String changes made by this patch: N/A
Attachment #600073 - Flags: approval-mozilla-aurora?
Comment on attachment 600073 [details] [diff] [review]

[Triage Comment]
Approved for Aurora 12 since this is low risk and we have heard of slow input into text boxes being an intermittent issue for our users.
Attachment #600073 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Is this something QA can verify?
Whiteboard: [qa?]
You need to log in before you can comment on or make changes to this bug.