Bug 82699

Summary: [Refactoring] Move Selection from DOMWindow to TreeScope
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: HTML EditingAssignee: Shinya Kawanaka <shinyak>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, enrica, gns, morrita, philn, rniwa, tkent, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 86172    
Bug Blocks: 82697, 82698    
Attachments:
Description Flags
WIP
none
WIP
none
WIP
none
Patch
none
Patch for landing
none
Patch none

Description Shinya Kawanaka 2012-03-29 23:49:38 PDT
Since ShadowRoot will also manage its own version of Selection, it would be beautiful if Selection is owned by TreeScope instead of DOMWindow.
Comment 1 Shinya Kawanaka 2012-05-09 23:07:35 PDT
Created attachment 141088 [details]
WIP
Comment 2 Early Warning System Bot 2012-05-09 23:22:33 PDT
Comment on attachment 141088 [details]
WIP

Attachment 141088 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12647828
Comment 3 Early Warning System Bot 2012-05-09 23:26:21 PDT
Comment on attachment 141088 [details]
WIP

Attachment 141088 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12646889
Comment 4 Build Bot 2012-05-09 23:32:44 PDT
Comment on attachment 141088 [details]
WIP

Attachment 141088 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12645954
Comment 5 Gyuyoung Kim 2012-05-09 23:33:20 PDT
Comment on attachment 141088 [details]
WIP

Attachment 141088 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12666058
Comment 6 Shinya Kawanaka 2012-05-09 23:40:38 PDT
Created attachment 141092 [details]
WIP
Comment 7 Build Bot 2012-05-10 00:01:25 PDT
Comment on attachment 141092 [details]
WIP

Attachment 141092 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12644954
Comment 8 Build Bot 2012-05-10 00:12:35 PDT
Comment on attachment 141092 [details]
WIP

Attachment 141092 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12649947
Comment 9 Early Warning System Bot 2012-05-10 00:15:14 PDT
Comment on attachment 141092 [details]
WIP

Attachment 141092 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12666066
Comment 10 Early Warning System Bot 2012-05-10 00:26:49 PDT
Comment on attachment 141092 [details]
WIP

Attachment 141092 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12653824
Comment 11 Shinya Kawanaka 2012-05-10 00:46:54 PDT
Created attachment 141102 [details]
WIP
Comment 12 Shinya Kawanaka 2012-05-10 01:37:32 PDT
Created attachment 141110 [details]
Patch
Comment 13 Ryosuke Niwa 2012-05-10 01:49:04 PDT
Comment on attachment 141110 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141110&action=review

> Source/WebCore/dom/TreeScope.cpp:60
> +        m_selection->disconnectTreeScope();

I would call it clearTreeScope.
Comment 14 Shinya Kawanaka 2012-05-10 01:57:27 PDT
(In reply to comment #13)
> (From update of attachment 141110 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141110&action=review
> 
> > Source/WebCore/dom/TreeScope.cpp:60
> > +        m_selection->disconnectTreeScope();
> 
> I would call it clearTreeScope.

Thank you for very fast reviewing!
I'll update the patch.
Comment 15 Shinya Kawanaka 2012-05-10 18:42:30 PDT
Created attachment 141309 [details]
Patch for landing
Comment 16 Shinya Kawanaka 2012-05-10 18:45:09 PDT
Committed r116715: <http://trac.webkit.org/changeset/116715>
Comment 17 WebKit Review Bot 2012-05-10 20:57:14 PDT
Re-opened since this is blocked by 86172
Comment 19 Shinya Kawanaka 2012-05-11 00:26:59 PDT
(In reply to comment #18)
> It seems that cross-frame-access-selection.html was broken by the patch.
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=http%2Ftests%2Fsecurity%2Fcross-frame-access-selection.html

Thank you for notifying that!

I've investigated the issue. Making DOMSelection non-DOMWindowProperty seems to cause that issue. I'll update the patch soon. I think this patch should be reviewed again.
Comment 20 Shinya Kawanaka 2012-05-11 00:29:02 PDT
Created attachment 141344 [details]
Patch
Comment 21 WebKit Review Bot 2012-05-11 03:35:49 PDT
Comment on attachment 141344 [details]
Patch

Clearing flags on attachment: 141344

Committed r116746: <http://trac.webkit.org/changeset/116746>
Comment 22 WebKit Review Bot 2012-05-11 03:35:55 PDT
All reviewed patches have been landed.  Closing bug.