Bug 52481

Summary: Stop instantiating legacy editing positions in AccessibilityRenderObject.cpp, Element.cpp, BreakBlockquoteCommand.cpp, CompositeEditCommand.cpp, and DeleteButtonController.cpp
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Enhancement CC: darin, eric, kalman, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 52099    
Attachments:
Description Flags
cleanup none

Ryosuke Niwa
Reported 2011-01-14 14:09:45 PST
This is a cleanup bug.
Attachments
cleanup (12.96 KB, patch)
2011-01-14 14:43 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2011-01-14 14:43:15 PST
Created attachment 79004 [details] cleanup
Eric Seidel (no email)
Comment 2 2011-01-14 14:44:55 PST
Comment on attachment 79004 [details] cleanup OK.
Eric Seidel (no email)
Comment 3 2011-01-14 14:45:24 PST
We're going to have to solve the Position ref-churn problem at some point. All these functions should be reutrning some sort of PassPosition object which knows how to avoid the ref-churn.
Ryosuke Niwa
Comment 4 2011-01-14 14:49:00 PST
(In reply to comment #3) > We're going to have to solve the Position ref-churn problem at some point. All these functions should be reutrning some sort of PassPosition object which knows how to avoid the ref-churn. Yeah, that'll be nice. In fact, PassPosition can just contain Node* and offset because we shouldn't be relying on positions to hang onto nodes given Position's constructor takes Node*.
Ryosuke Niwa
Comment 5 2011-01-14 16:17:00 PST
Comment on attachment 79004 [details] cleanup Clearing flags on attachment: 79004 Committed r75835: <http://trac.webkit.org/changeset/75835>
Ryosuke Niwa
Comment 6 2011-01-14 16:17:05 PST
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 7 2011-01-17 12:57:46 PST
(In reply to comment #4) > PassPosition can just contain Node* and offset because we shouldn't be relying on positions to hang onto nodes given Position's constructor takes Node*. That’t not right. I think PassPosition needs to contain a RefPtr or PassRefPtr.
Ryosuke Niwa
Comment 8 2011-01-17 13:02:12 PST
(In reply to comment #7) > (In reply to comment #4) > > PassPosition can just contain Node* and offset because we shouldn't be relying on positions to hang onto nodes given Position's constructor takes Node*. > > That’t not right. I think PassPosition needs to contain a RefPtr or PassRefPtr. Yeah, it would have worked if only Position's helper functions (which also take Node*) returned PassPosition but to allow other functions return PassPosition, we need to use PassRefPtr (my work-in-progress patch uses PassRefPtr). Anyhow, PassPosition is to be implemented in the bug 52504.
Note You need to log in before you can comment on or make changes to this bug.