Bug 82699

Summary: [Refactoring] Move Selection from DOMWindow to TreeScope
Product: WebKit Reporter: Shinya Kawanaka <shinyak@chromium.org>
Component: HTML EditingAssignee: Shinya Kawanaka <shinyak@chromium.org>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov@chromium.org, enrica@apple.com, gns@gnome.org, morrita@google.com, philn@igalia.com, rniwa@webkit.org, tkent@chromium.org, webkit.review.bot@gmail.com, xan.lopez@gmail.com
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 From 2012-03-29 23:49:38 PST
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 From 2012-05-09 23:07:35 PST -------
Created an attachment (id=141088) [details]
WIP
------- Comment #2 From 2012-05-09 23:22:33 PST -------
(From update of attachment 141088 [details])
Attachment 141088 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12647828
------- Comment #3 From 2012-05-09 23:26:21 PST -------
(From update of attachment 141088 [details])
Attachment 141088 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12646889
------- Comment #4 From 2012-05-09 23:32:44 PST -------
(From update of attachment 141088 [details])
Attachment 141088 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12645954
------- Comment #5 From 2012-05-09 23:33:20 PST -------
(From update of attachment 141088 [details])
Attachment 141088 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12666058
------- Comment #6 From 2012-05-09 23:40:38 PST -------
Created an attachment (id=141092) [details]
WIP
------- Comment #7 From 2012-05-10 00:01:25 PST -------
(From update of attachment 141092 [details])
Attachment 141092 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12644954
------- Comment #8 From 2012-05-10 00:12:35 PST -------
(From update of attachment 141092 [details])
Attachment 141092 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12649947
------- Comment #9 From 2012-05-10 00:15:14 PST -------
(From update of attachment 141092 [details])
Attachment 141092 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12666066
------- Comment #10 From 2012-05-10 00:26:49 PST -------
(From update of attachment 141092 [details])
Attachment 141092 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12653824
------- Comment #11 From 2012-05-10 00:46:54 PST -------
Created an attachment (id=141102) [details]
WIP
------- Comment #12 From 2012-05-10 01:37:32 PST -------
Created an attachment (id=141110) [details]
Patch
------- Comment #13 From 2012-05-10 01:49:04 PST -------
(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.
------- Comment #14 From 2012-05-10 01:57:27 PST -------
(In reply to comment #13)
> (From update of attachment 141110 [details] [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 From 2012-05-10 18:42:30 PST -------
Created an attachment (id=141309) [details]
Patch for landing
------- Comment #16 From 2012-05-10 18:45:09 PST -------
Committed r116715: <http://trac.webkit.org/changeset/116715>
------- Comment #17 From 2012-05-10 20:57:14 PST -------
Re-opened since this is blocked by 86172
------- Comment #18 From 2012-05-10 21:00:10 PST -------
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
------- Comment #19 From 2012-05-11 00:26:59 PST -------
(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 From 2012-05-11 00:29:02 PST -------
Created an attachment (id=141344) [details]
Patch
------- Comment #21 From 2012-05-11 03:35:49 PST -------
(From update of attachment 141344 [details])
Clearing flags on attachment: 141344

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