Bug 108209 - Refactor ShadowRoot exception handling
Summary: Refactor ShadowRoot exception handling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Elliott Sprehn
URL:
Keywords:
Depends on:
Blocks: 108301
  Show dependency tree / graph
 
Reported: 2013-01-29 11:32 PST by Elliott Sprehn
Modified: 2013-01-30 00:48 PST (History)
12 users (show)

See Also:

Attachments
Patch (9.86 KB, patch)
2013-01-29 11:37 PST, Elliott Sprehn 		no flags Details | Formatted Diff | Diff
Patch (12.85 KB, patch)
2013-01-29 12:24 PST, Elliott Sprehn 		no flags Details | Formatted Diff | Diff
Patch (18.61 KB, patch)
2013-01-29 13:06 PST, Elliott Sprehn 		no flags Details | Formatted Diff | Diff
Patch (19.32 KB, patch)
2013-01-29 13:47 PST, Elliott Sprehn 		no flags Details | Formatted Diff | Diff
Patch (19.55 KB, patch)
2013-01-29 14:12 PST, Elliott Sprehn 		no flags Details | Formatted Diff | Diff
Patch (19.34 KB, patch)
2013-01-29 15:26 PST, Elliott Sprehn 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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