WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
78453
Needs an internal flag to accept multiple shadow roots for the purpose of tests
https://bugs.webkit.org/show_bug.cgi?id=78453
Summary
Needs an internal flag to accept multiple shadow roots for the purpose of tests
Hayato Ito
Reported
2012-02-12 20:28:22 PST
We need a simple internal flag on ShadowRootList by which it accepts multiple shadow tree. We should avoid accepting multiple shadow roots until multiple shadow tree becomes stable. At the same time, we need to test multiple shadow tree. Therefore it might be better to have a flag (maybe static bool member variable is enough for the purpose of test) and expose the setter through window.internals.
Attachments
add flag
(7.92 KB, patch)
2012-02-12 22:13 PST
,
Hayato Ito
no flags
Details
Formatted Diff
Diff
fix build on mac, gtk, win (hopefully)
(12.53 KB, patch)
2012-02-13 19:50 PST
,
Hayato Ito
no flags
Details
Formatted Diff
Diff
turn flag off after a test.
(12.59 KB, patch)
2012-02-13 22:47 PST
,
Hayato Ito
no flags
Details
Formatted Diff
Diff
Patch
(12.12 KB, patch)
2012-02-15 18:38 PST
,
Shinya Kawanaka
no flags
Details
Formatted Diff
Diff
Test
(7.32 KB, patch)
2012-02-15 23:40 PST
,
Shinya Kawanaka
no flags
Details
Formatted Diff
Diff
Patch
(9.61 KB, patch)
2012-02-16 00:17 PST
,
Shinya Kawanaka
no flags
Details
Formatted Diff
Diff
Patch
(9.62 KB, patch)
2012-02-16 00:19 PST
,
Shinya Kawanaka
no flags
Details
Formatted Diff
Diff
Patch
(9.62 KB, patch)
2012-02-16 01:17 PST
,
Shinya Kawanaka
morrita
: review+
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Hayato Ito
Comment 1
2012-02-12 22:13:09 PST
Created
attachment 126715
[details]
add flag
Philippe Normand
Comment 2
2012-02-12 22:19:58 PST
Comment on
attachment 126715
[details]
add flag
Attachment 126715
[details]
did not pass gtk-ews (gtk): Output:
http://queues.webkit.org/results/11508667
Hayato Ito
Comment 3
2012-02-12 22:23:34 PST
We need to add symbols for some ports. Let me watch the result of build on some ports.
Hayato Ito
Comment 4
2012-02-13 00:21:30 PST
windows build bot seems down.
Hayato Ito
Comment 5
2012-02-13 19:50:48 PST
Created
attachment 126894
[details]
fix build on mac, gtk, win (hopefully)
Shinya Kawanaka
Comment 6
2012-02-13 22:27:47 PST
Comment on
attachment 126894
[details]
fix build on mac, gtk, win (hopefully) View in context:
https://bugs.webkit.org/attachment.cgi?id=126894&action=review
> LayoutTests/fast/dom/shadow/shadow-root-js-api.html:62 > + window.internals.setMultipleShadowRootsEnabled(true);
Could you make this flag OFF after the test? This has a side effect actually, and will cause flaky results.
Hayato Ito
Comment 7
2012-02-13 22:47:29 PST
Created
attachment 126911
[details]
turn flag off after a test.
WebKit Review Bot
Comment 8
2012-02-13 22:49:10 PST
Attachment 126911
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Last 3072 characters of output: fast/backgrounds/size/backgroundSize21-expected.txt W: -empty_dir: trunk/LayoutTests/platform/qt/fast/backgrounds/size/backgroundSize22-expected.txt
r107672
= 444e12b7089743ee976e71dd7889fca2dd0c50b0 (refs/remotes/origin/master) First, rewinding head to replay your work on top of it... Applying: [chromium] Rebaseline JPEG image results after
r107389
Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... warning: too many files (created: 168845 deleted: 8), skipping inexact rename detection error: refusing to lose untracked file at 'LayoutTests/fast/backgrounds/size/backgroundSize21-expected.txt' error: refusing to lose untracked file at 'LayoutTests/fast/backgrounds/size/backgroundSize22-expected.txt' CONFLICT (rename/rename): Rename "LayoutTests/platform/gtk/fast/backgrounds/size/backgroundSize21-expected.txt"->"LayoutTests/platform/efl/fast/backgrounds/size/backgroundSize21-expected.txt" in branch "HEAD" rename "LayoutTests/platform/gtk/fast/backgrounds/size/backgroundSize21-expected.txt"->"LayoutTests/fast/backgrounds/size/backgroundSize21-expected.txt" in "[chromium] Rebaseline JPEG image results after
r107389
" CONFLICT (rename/rename): Rename "LayoutTests/platform/gtk/fast/backgrounds/size/backgroundSize22-expected.txt"->"LayoutTests/platform/efl/fast/backgrounds/size/backgroundSize22-expected.txt" in branch "HEAD" rename "LayoutTests/platform/gtk/fast/backgrounds/size/backgroundSize22-expected.txt"->"LayoutTests/fast/backgrounds/size/backgroundSize22-expected.txt" in "[chromium] Rebaseline JPEG image results after
r107389
" CONFLICT (rename/delete): Rename LayoutTests/platform/mac/fast/backgrounds/size/backgroundSize21-expected.txt->LayoutTests/fast/backgrounds/size/backgroundSize19-expected.txt in HEAD and deleted in [chromium] Rebaseline JPEG image results after
r107389
CONFLICT (rename/delete): Rename LayoutTests/platform/mac/fast/backgrounds/size/backgroundSize22-expected.txt->LayoutTests/fast/backgrounds/size/backgroundSize20-expected.txt in HEAD and deleted in [chromium] Rebaseline JPEG image results after
r107389
CONFLICT (rename/delete): Rename LayoutTests/platform/qt/fast/backgrounds/size/backgroundSize21-expected.txt->LayoutTests/fast/backgrounds/size/backgroundSize21-expected.txt in HEAD and deleted in [chromium] Rebaseline JPEG image results after
r107389
CONFLICT (rename/delete): Rename LayoutTests/platform/qt/fast/backgrounds/size/backgroundSize22-expected.txt->LayoutTests/fast/backgrounds/size/backgroundSize22-expected.txt in HEAD and deleted in [chromium] Rebaseline JPEG image results after
r107389
Auto-merging LayoutTests/ChangeLog Failed to merge in the changes. Patch failed at 0001 [chromium] Rebaseline JPEG image results after
r107389
When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.
Hayato Ito
Comment 9
2012-02-13 22:51:40 PST
Nice catch. Done. I am afraid that it is dangerous to use static variable flag. We should be careful for writing tests using this flag. (In reply to
comment #6
)
> (From update of
attachment 126894
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=126894&action=review
> > > LayoutTests/fast/dom/shadow/shadow-root-js-api.html:62 > > + window.internals.setMultipleShadowRootsEnabled(true); > > Could you make this flag OFF after the test? This has a side effect actually, and will cause flaky results.
Hajime Morrita
Comment 10
2012-02-13 23:35:15 PST
Do we really need this ?
Hayato Ito
Comment 11
2012-02-13 23:46:42 PST
(In reply to
comment #10
)
> Do we really need this ?
As discussed with shinyak, we need this to move forward step by step. We need to write tests using this flag for multiple shadow trees. But we don't want to accept multiple shadow roots for a while in production, which will be the root cause of crashes.
Shinya Kawanaka
Comment 12
2012-02-13 23:58:14 PST
(In reply to
comment #11
)
> (In reply to
comment #10
) > > Do we really need this ? > > As discussed with shinyak, we need this to move forward step by step. > We need to write tests using this flag for multiple shadow trees. But we don't want to accept multiple shadow roots for a while in production, which will be the root cause of crashes.
Yeah, I agree. I think multiple shadow subtrees will be unstable for a while, and it will take time to implement it stably. It's good to have a flag for us to go forward step by step.
Shinya Kawanaka
Comment 13
2012-02-15 17:33:27 PST
Since hayato-san is off a few days, I'll take over this patch. I need this patch.
Shinya Kawanaka
Comment 14
2012-02-15 18:38:42 PST
Created
attachment 127290
[details]
Patch
Shinya Kawanaka
Comment 15
2012-02-15 23:40:41 PST
Created
attachment 127320
[details]
Test
Shinya Kawanaka
Comment 16
2012-02-16 00:17:27 PST
Created
attachment 127322
[details]
Patch
Shinya Kawanaka
Comment 17
2012-02-16 00:19:29 PST
Created
attachment 127323
[details]
Patch
Early Warning System Bot
Comment 18
2012-02-16 01:05:49 PST
Comment on
attachment 127323
[details]
Patch
Attachment 127323
[details]
did not pass qt-ews (qt): Output:
http://queues.webkit.org/results/11530990
Shinya Kawanaka
Comment 19
2012-02-16 01:17:42 PST
Created
attachment 127328
[details]
Patch
Shinya Kawanaka
Comment 20
2012-02-16 16:57:47 PST
Committed
r108003
: <
http://trac.webkit.org/changeset/108003
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug