Bug 108209

Summary: Refactor ShadowRoot exception handling
Product: WebKit Reporter: Elliott Sprehn <esprehn>
Component: New BugsAssignee: Elliott Sprehn <esprehn>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cmarcelo, dglazkov, mifenton, morrita, ojan.autocc, ojan, peter+ews, rniwa, tkent, webkit-ews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108301    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Elliott Sprehn 2013-01-29 11:32:25 PST 
Refactor ShadowRoot exception handling
Comment 1 Elliott Sprehn 2013-01-29 11:37:05 PST 
Created attachment 185275 [details]
Patch
Comment 2 Early Warning System Bot 2013-01-29 11:41:25 PST 
Comment on attachment 185275 [details]
Patch

Attachment 185275 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16195241
Comment 3 Early Warning System Bot 2013-01-29 11:44:01 PST 
Comment on attachment 185275 [details]
Patch

Attachment 185275 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16187316
Comment 4 EFL EWS Bot 2013-01-29 11:47:00 PST 
Comment on attachment 185275 [details]
Patch

Attachment 185275 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16151309
Comment 5 WebKit Review Bot 2013-01-29 11:54:23 PST 
Comment on attachment 185275 [details]
Patch

Attachment 185275 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16151315
Comment 6 Elliott Sprehn 2013-01-29 12:00:49 PST 
Woops, it seems we do ShadowRoot::create in a bunch of places I missed.
Comment 7 Build Bot 2013-01-29 12:01:07 PST 
Comment on attachment 185275 [details]
Patch

Attachment 185275 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16198259
Comment 8 Peter Beverloo (cr-android ews) 2013-01-29 12:03:38 PST 
Comment on attachment 185275 [details]
Patch

Attachment 185275 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/16200266
Comment 9 Build Bot 2013-01-29 12:11:19 PST 
Comment on attachment 185275 [details]
Patch

Attachment 185275 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16202258
Comment 10 Elliott Sprehn 2013-01-29 12:24:53 PST 
Created attachment 185283 [details]
Patch
Comment 11 Elliott Sprehn 2013-01-29 12:26:22 PST 
(In reply to comment #10)
> Created an attachment (id=185283) [details]
> Patch

Fixed the build, should be good for review now.
Comment 12 Build Bot 2013-01-29 12:51:01 PST 
Comment on attachment 185283 [details]
Patch

Attachment 185283 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16183293
Comment 13 Build Bot 2013-01-29 12:56:37 PST 
Comment on attachment 185283 [details]
Patch

Attachment 185283 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16200292
Comment 14 Elliott Sprehn 2013-01-29 13:06:31 PST 
Created attachment 185292 [details]
Patch

Exports, yay.
Comment 15 Build Bot 2013-01-29 13:36:35 PST 
Comment on attachment 185292 [details]
Patch

Attachment 185292 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16183309
Comment 16 Build Bot 2013-01-29 13:43:32 PST 
Comment on attachment 185292 [details]
Patch

Attachment 185292 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16198296
Comment 17 Elliott Sprehn 2013-01-29 13:47:52 PST 
Created attachment 185300 [details]
Patch

More exporting.
Comment 18 Elliott Sprehn 2013-01-29 14:12:49 PST 
Created attachment 185306 [details]
Patch
Comment 19 Elliott Sprehn 2013-01-29 15:26:20 PST 
Created attachment 185319 [details]
Patch
Comment 20 WebKit Review Bot 2013-01-29 16:03:48 PST 
Comment on attachment 185319 [details]
Patch

Clearing flags on attachment: 185319

Committed r141175: <http://trac.webkit.org/changeset/141175>
Comment 21 WebKit Review Bot 2013-01-29 16:03:55 PST 
All reviewed patches have been landed.  Closing bug.
Comment 22 Elliott Sprehn 2013-01-30 00:00:04 PST 
Reading the code more closely it appears this patch should have broken TextFieldDecorationElement because it intended to actually create multiple user agent shadow roots!??

Unfortunately there's no layout tests for this code, in fact TextFieldDecorationElement is only used in ChromeClientImpl::addTextFieldDecorationsTo which seems really bad that we have magical elements and hacks in WebCore that are specifically for Chromium and then have no tests in WebCore for them.
Comment 23 Elliott Sprehn 2013-01-30 00:47:59 PST 
(In reply to comment #22)
> Reading the code more closely it appears this patch should have broken TextFieldDecorationElement because it intended to actually create multiple user agent shadow roots!??
> 
> Unfortunately there's no layout tests for this code, in fact TextFieldDecorationElement is only used in ChromeClientImpl::addTextFieldDecorationsTo which seems really bad that we have magical elements and hacks in WebCore that are specifically for Chromium and then have no tests in WebCore for them.

https://bugs.webkit.org/show_bug.cgi?id=108301