Bug 79768 - Methods in ShadowTree and TreeScopeAdopter should be multiple shadow roots aware.
Summary: Methods in ShadowTree and TreeScopeAdopter should be multiple shadow roots aw...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Shinya Kawanaka
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-28 02:06 PST by Roland Steiner
Modified: 2012-02-29 17:55 PST (History)
4 users (show)

See Also:


Attachments
Patch (12.81 KB, patch)
2012-02-29 00:59 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch (12.82 KB, patch)
2012-02-29 01:05 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Wrong patch (11.12 KB, patch)
2012-02-29 03:32 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch for landing (12.78 KB, patch)
2012-02-29 17:27 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff
Patch for landing (12.78 KB, patch)
2012-02-29 17:33 PST, Shinya Kawanaka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Steiner 2012-02-28 02:06:16 PST
ShadowTree::recalcShadowTreeStyle only updates the youngest tree - shouldn't it update all?
Comment 1 Shinya Kawanaka 2012-02-28 04:48:35 PST
Thanks for the report! I'll check it later.
Comment 2 Hajime Morrita 2012-02-28 22:45:07 PST
Searching youngestShadowRoot(), it turns out some other functions
like following is also need to be multiple-shadow-aware.

- bool ShadowTree::needsStyleRecalc()  
- bool ShadowTree::childNeedsStyleRecalc() 
- bool ShadowTree::needsReattachHostChildrenAndShadow()
- void ShadowTree::hostChildrenChanged()  
- void TreeScopeAdopter::moveTreeToNewScope(Node* root)
- void TreeScopeAdopter::moveTreeToNewDocument(Node* root, Document* oldDocument, Document* newDocument) const
Comment 3 Shinya Kawanaka 2012-02-28 23:59:43 PST
(In reply to comment #2)
> Searching youngestShadowRoot(), it turns out some other functions
> like following is also need to be multiple-shadow-aware.
> 
> - bool ShadowTree::needsStyleRecalc()  
> - bool ShadowTree::childNeedsStyleRecalc() 
> - bool ShadowTree::needsReattachHostChildrenAndShadow()
> - void ShadowTree::hostChildrenChanged()  
> - void TreeScopeAdopter::moveTreeToNewScope(Node* root)
> - void TreeScopeAdopter::moveTreeToNewDocument(Node* root, Document* oldDocument, Document* newDocument) const

I think we should consider multiple shadow subtrees for needsStyleRecalc() and childNeedsStyleRecalc(),
but not for needsReattachHostChildrenAndShadow() and needsReattachHostChildrenAndShadow(),
because the latter methods checks the existence of InsertionPoint of the youngest shadow roots. If there is no insertion point in the youngest shadow root, older shadow subtrees or light children won't be rendered al all.
If this assumption is wrong, please let me know...
Comment 4 Shinya Kawanaka 2012-02-29 00:59:58 PST
Created attachment 129407 [details]
Patch
Comment 5 Shinya Kawanaka 2012-02-29 01:05:14 PST
Created attachment 129408 [details]
Patch
Comment 6 WebKit Review Bot 2012-02-29 01:08:13 PST
Attachment 129408 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
Index mismatch: 45f1ddc97c26c8f168daebf1d8d25e32afde0c7a != d0c2d29a78e61f43d073201c516920e3103f993d
rereading 21dd0231e92234ee59517801b9836dc9d53fe30a
	M	Source/WebCore/dom/ShadowTree.h
	M	Source/WebCore/dom/Element.cpp
	M	Source/WebCore/dom/ShadowTree.cpp
	M	Source/WebCore/dom/ContainerNode.cpp
	M	Source/WebCore/dom/ContainerNode.h
	M	Source/WebCore/ChangeLog
109203 = f269c80d877021b19c2311e4e7c7a1dc9d95a3b8 already exists! Why are we refetching it?
 at /usr/lib/git-core/git-svn line 5210

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 WebKit Review Bot 2012-02-29 03:02:54 PST
Comment on attachment 129408 [details]
Patch

Rejecting attachment 129408 [details] from commit-queue.

shinyak@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 8 Shinya Kawanaka 2012-02-29 03:32:59 PST
Created attachment 129435 [details]
Wrong patch
Comment 9 Shinya Kawanaka 2012-02-29 17:20:30 PST
(In reply to comment #8)
> Created an attachment (id=129435) [details]
> Patch

Sorry, I mistook the place to upload...
Comment 10 Shinya Kawanaka 2012-02-29 17:27:55 PST
Created attachment 129561 [details]
Patch for landing
Comment 11 Shinya Kawanaka 2012-02-29 17:33:42 PST
Created attachment 129563 [details]
Patch for landing
Comment 12 Shinya Kawanaka 2012-02-29 17:55:25 PST
Committed r109290: <http://trac.webkit.org/changeset/109290>