Bug 191237 - Remove SVG properties tear-off objects
Summary: Remove SVG properties tear-off objects
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
: 91191 132557 135040 168586 173178 184670 191047 191372 (view as bug list)
Depends on: 195722 195859 195862 195863 195905 195949 195960 196025 196037 196065 196083 196084 196085 196086 196087 196263
Blocks: 191292 150388
  Show dependency treegraph
 
Reported: 2018-11-04 14:37 PST by Said Abou-Hallawa
Modified: 2019-05-17 09:59 PDT (History)
18 users (show)

See Also:


Attachments
Patch (1.44 MB, patch)
2018-11-04 15:02 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.44 MB, patch)
2018-11-04 16:41 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.43 MB, application/zip)
2018-11-04 17:54 PST, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.14 MB, application/zip)
2018-11-04 18:05 PST, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-sierra (3.11 MB, application/zip)
2018-11-04 18:48 PST, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (9.81 MB, application/zip)
2018-11-04 18:59 PST, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-sierra (3.15 MB, application/zip)
2018-11-04 20:51 PST, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (2.41 MB, application/zip)
2018-11-04 20:54 PST, Build Bot
no flags Details
Patch (1.53 MB, patch)
2018-11-20 21:13 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.53 MB, patch)
2018-11-20 22:07 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.53 MB, patch)
2018-11-20 22:47 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.43 MB, application/zip)
2018-11-21 00:58 PST, Build Bot
no flags Details
Patch (1.53 MB, patch)
2018-11-24 18:31 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.53 MB, patch)
2018-11-24 20:27 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.53 MB, patch)
2018-11-24 20:47 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.53 MB, patch)
2018-11-24 21:03 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.53 MB, patch)
2018-11-24 21:19 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.53 MB, patch)
2018-11-24 21:50 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.45 MB, application/zip)
2018-11-25 00:07 PST, Build Bot
no flags Details
Patch (1.50 MB, patch)
2018-11-26 12:17 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.50 MB, patch)
2018-11-26 13:55 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.50 MB, patch)
2018-11-26 16:34 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.50 MB, patch)
2018-11-26 18:06 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.39 MB, application/zip)
2018-11-26 21:31 PST, Build Bot
no flags Details
Patch (1.50 MB, patch)
2018-11-27 11:42 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews206 for win-future (12.81 MB, application/zip)
2018-11-27 14:05 PST, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.90 MB, application/zip)
2018-11-27 16:46 PST, Build Bot
no flags Details
Patch (1.55 MB, patch)
2018-12-02 02:18 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.55 MB, patch)
2018-12-02 03:40 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.62 MB, application/zip)
2018-12-02 08:07 PST, Build Bot
no flags Details
Patch (1.55 MB, patch)
2018-12-02 09:42 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.55 MB, patch)
2018-12-02 11:11 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.55 MB, patch)
2018-12-02 11:36 PST, Said Abou-Hallawa
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews200 for win-future (12.81 MB, application/zip)
2018-12-02 13:29 PST, Build Bot
no flags Details
1-SVGProperty (92.83 KB, patch)
2018-12-02 17:19 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
2-SVGPathSeg (96.37 KB, patch)
2018-12-02 18:24 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
3-SVGList (75.45 KB, patch)
2018-12-02 23:31 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.56 MB, application/zip)
2018-12-02 23:59 PST, Build Bot
no flags Details
4-SVGDecoratedEnumeration (11.44 KB, patch)
2018-12-03 08:00 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
5-SVGPathSegList (42.74 KB, patch)
2018-12-04 07:56 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
6-SVGAnimatedProperty (42.31 KB, patch)
2018-12-04 08:59 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
7-SVGAnimationFunction (54.00 KB, patch)
2018-12-04 09:45 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
8-SVGAnimator.patch (91.91 KB, patch)
2018-12-04 21:09 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
9-SVGAccessor (48.16 KB, patch)
2018-12-05 08:28 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
10-SVGPropertyRegistry (21.78 KB, patch)
2018-12-06 08:01 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
11-JSBinding (5.37 KB, patch)
2018-12-06 08:02 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
12-FilesRemoval (301.39 KB, patch)
2018-12-06 08:02 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
13-BasicShapes (62.18 KB, patch)
2018-12-06 08:03 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
14-Resources (74.86 KB, patch)
2018-12-06 08:03 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
15-Filters (187.50 KB, patch)
2018-12-06 08:04 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
16-Text (28.05 KB, patch)
2018-12-06 14:04 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
17-BaseTypes (72.00 KB, patch)
2018-12-07 07:47 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
18-Conatiners (43.94 KB, patch)
2018-12-07 08:13 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
19-SVGAnimateElement (57.92 KB, patch)
2018-12-08 11:33 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
20-Miscellaneous (25.90 KB, patch)
2018-12-08 11:33 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
21-Extra (89.89 KB, patch)
2018-12-08 11:34 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
22-LayoutTests (62.60 KB, patch)
2018-12-08 11:34 PST, Said Abou-Hallawa
ews: commit-queue-
Details | Formatted Diff | Diff
EWSPatch (1.60 MB, patch)
2018-12-08 11:38 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-sierra (4.37 MB, application/zip)
2018-12-08 12:43 PST, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-sierra-wk2 (4.71 MB, application/zip)
2018-12-08 12:49 PST, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (4.32 MB, application/zip)
2018-12-08 13:28 PST, Build Bot
no flags Details
Archive of layout-test-results from ews203 for win-future (13.95 MB, application/zip)
2018-12-08 13:30 PST, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-sierra (3.70 MB, application/zip)
2018-12-08 13:32 PST, Build Bot
no flags Details
EWSPatch (1.50 MB, patch)
2018-12-08 16:24 PST, Said Abou-Hallawa
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews200 for win-future (12.79 MB, application/zip)
2018-12-08 18:17 PST, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.48 MB, application/zip)
2018-12-08 18:26 PST, Build Bot
no flags Details
EWSPatch (1.59 MB, patch)
2019-01-22 00:24 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
EWSPatch (1.58 MB, patch)
2019-01-22 07:59 PST, Said Abou-Hallawa
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (2.46 MB, application/zip)
2019-01-22 10:16 PST, Build Bot
no flags Details
Rebased EWSPatch (1.50 MB, patch)
2019-03-04 18:42 PST, Said Abou-Hallawa
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews201 for win-future (12.85 MB, application/zip)
2019-03-04 20:50 PST, Build Bot
no flags Details
Archive of layout-test-results from ews126 for ios-simulator-wk2 (8.91 MB, application/zip)
2019-03-05 00:36 PST, Build Bot
no flags Details
Patch for review (1.60 MB, patch)
2019-03-05 14:33 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Unified Patch (1.57 MB, patch)
2019-03-09 23:26 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (2.55 MB, application/zip)
2019-03-10 01:26 PST, Build Bot
no flags Details
Patch (256.88 KB, patch)
2019-04-02 11:36 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (258.37 KB, patch)
2019-04-02 17:18 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (297.36 KB, patch)
2019-04-03 13:43 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2018-11-04 14:37:58 PST
SVG properties tear-off objects have been troublesome and confusing part of SVG. These objects are created only when requested from DOM and SMIL animations. They wrap primitive or value members in the SVGElements. Their life cycle requires the SVGElement and all the objects in between to be alive even if they are not accessible from the anything else. Many things can go wrong trying to strongly tie the tear-off object with the SVGElement. Trying to fix these issues by relaxing the relationship leads to different kind of problems.

The current code

-- Does not reflect the specs which makes difficult to correlate between them.   
-- Lacks clear object life cycle, see all different kinds of pointes in SVGAnimatedProperty, SVGAnimatedPropertyTearOff and SVGAnimatedListPropertyTearOff
-- Lacks correct encapsulation, see the SVGPathElement, SVGAnimatedPathSegList and SVGPathSegList
-- Lacks strong type variable declaration, see SVGAnimatedType
-- Does not choose the right time and place to extract information, see many functions in SVGElement which includes HashMap or HashSet of attributes' names. See SVGAnimateElementBase::determineAnimatedPropertyType() and SVGAnimationElement::checkInvalidCSSAttributeType()
-- Have hacks to overcome the limitation of tear-off object creation and caching, see SVGTests::requiredFeatures() and SVGViewSpec::transform()
-- Has loose relationship between the animator and the form and the to values. See SVGAnimateElementBase.
-- Have correctness issues when animating the same attribute with multiple animators. See https://bugs.webkit.org/show_bug.cgi?id=150388.
-- Have correctness issues with optional attributes, see https://bugs.webkit.org/show_bug.cgi?id=154141.
Comment 1 Said Abou-Hallawa 2018-11-04 15:02:45 PST
Created attachment 353808 [details]
Patch
Comment 2 Build Bot 2018-11-04 15:07:50 PST
Attachment 353808 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:150:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 234 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Said Abou-Hallawa 2018-11-04 16:41:58 PST
Created attachment 353812 [details]
Patch
Comment 4 Build Bot 2018-11-04 16:46:23 PST
Attachment 353812 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:150:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 234 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Build Bot 2018-11-04 17:53:58 PST
Comment on attachment 353812 [details]
Patch

Attachment 353812 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9858568

New failing tests:
svg/W3C-SVG-1.1/animate-elem-30-t.svg
Comment 6 Build Bot 2018-11-04 17:54:00 PST
Created attachment 353815 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 7 Build Bot 2018-11-04 18:04:59 PST
Comment on attachment 353812 [details]
Patch

Attachment 353812 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9858574

New failing tests:
svg/W3C-SVG-1.1/animate-elem-30-t.svg
Comment 8 Build Bot 2018-11-04 18:05:01 PST
Created attachment 353817 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 9 Build Bot 2018-11-04 18:48:14 PST
Comment on attachment 353812 [details]
Patch

Attachment 353812 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9858695

New failing tests:
svg/W3C-SVG-1.1/animate-elem-30-t.svg
Comment 10 Build Bot 2018-11-04 18:48:16 PST
Created attachment 353818 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 11 Build Bot 2018-11-04 18:59:41 PST
Comment on attachment 353812 [details]
Patch

Attachment 353812 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9858742

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
svg/W3C-SVG-1.1/animate-elem-30-t.svg
perf/svg-path-appenditem.html
Comment 12 Build Bot 2018-11-04 18:59:43 PST
Created attachment 353820 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 13 Build Bot 2018-11-04 20:50:59 PST
Comment on attachment 353812 [details]
Patch

Attachment 353812 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9859515

New failing tests:
svg/W3C-SVG-1.1/animate-elem-30-t.svg
Comment 14 Build Bot 2018-11-04 20:51:01 PST
Created attachment 353824 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 15 Build Bot 2018-11-04 20:54:17 PST
Comment on attachment 353812 [details]
Patch

Attachment 353812 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9859516

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
svg/W3C-SVG-1.1/animate-elem-30-t.svg
perf/svg-path-appenditem.html
Comment 16 Build Bot 2018-11-04 20:54:19 PST
Created attachment 353825 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 17 Radar WebKit Bug Importer 2018-11-07 13:45:13 PST
<rdar://problem/45887675>
Comment 18 Said Abou-Hallawa 2018-11-07 13:49:16 PST
*** Bug 191047 has been marked as a duplicate of this bug. ***
Comment 19 Said Abou-Hallawa 2018-11-07 13:49:52 PST
*** Bug 173178 has been marked as a duplicate of this bug. ***
Comment 20 Said Abou-Hallawa 2018-11-20 21:13:49 PST
Created attachment 355382 [details]
Patch
Comment 21 Build Bot 2018-11-20 21:17:37 PST
Attachment 355382 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 268 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 Said Abou-Hallawa 2018-11-20 22:07:13 PST
Created attachment 355386 [details]
Patch
Comment 23 Build Bot 2018-11-20 22:11:06 PST
Attachment 355386 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 268 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Said Abou-Hallawa 2018-11-20 22:47:07 PST
Created attachment 355388 [details]
Patch
Comment 25 Build Bot 2018-11-20 22:50:55 PST
Attachment 355388 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 268 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 Build Bot 2018-11-21 00:58:01 PST
Comment on attachment 355388 [details]
Patch

Attachment 355388 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10096337

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
Comment 27 Build Bot 2018-11-21 00:58:03 PST
Created attachment 355393 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 28 Dirk Schulze 2018-11-21 01:18:32 PST
From the comment it was not entirely clear:
* if you remove SVG DOM or the Amin part of it? W/o replacement?
* SMIL continues to work.

SVG DOM animVal is discontinued by SVG WG. So that part would be fine I guess. For everything else I suggest informing WebKit-dev.
Comment 29 Said Abou-Hallawa 2018-11-24 18:25:03 PST
(In reply to Dirk Schulze from comment #28)
> From the comment it was not entirely clear:
> * if you remove SVG DOM or the Amin part of it? W/o replacement?

I am replacing the SVG properties and the SVG animated properties by proprieties owned by the SVG elements directly instead of the tear-off objects.

> * SMIL continues to work.
> 

Yes.

> SVG DOM animVal is discontinued by SVG WG. So that part would be fine I
> guess. For everything else I suggest informing WebKit-dev.

No. I did not remove the animVal because this would require a big change in the layout tests. The only SVG2 change, I did in this patch, was the insertion of non detached items in SVGLists. I will send an email to webkit-dev once a plan for landing this patch is determined.

Thanks.
Comment 30 Said Abou-Hallawa 2018-11-24 18:31:43 PST
Created attachment 355568 [details]
Patch
Comment 31 Said Abou-Hallawa 2018-11-24 20:27:25 PST
Created attachment 355574 [details]
Patch
Comment 32 Build Bot 2018-11-24 20:32:29 PST
Attachment 355574 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 268 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 33 Said Abou-Hallawa 2018-11-24 20:47:26 PST
Created attachment 355575 [details]
Patch
Comment 34 Build Bot 2018-11-24 20:52:09 PST
Attachment 355575 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 268 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 35 Said Abou-Hallawa 2018-11-24 21:03:44 PST
Created attachment 355577 [details]
Patch
Comment 36 Build Bot 2018-11-24 21:08:07 PST
Attachment 355577 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 268 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 37 Said Abou-Hallawa 2018-11-24 21:19:19 PST
Created attachment 355580 [details]
Patch
Comment 38 Build Bot 2018-11-24 21:22:49 PST
Attachment 355580 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 268 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 39 Said Abou-Hallawa 2018-11-24 21:50:26 PST
Created attachment 355582 [details]
Patch
Comment 40 Build Bot 2018-11-24 21:54:15 PST
Attachment 355582 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 268 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 41 Build Bot 2018-11-25 00:07:40 PST
Comment on attachment 355582 [details]
Patch

Attachment 355582 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10138151

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
Comment 42 Build Bot 2018-11-25 00:07:42 PST
Created attachment 355585 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 43 Said Abou-Hallawa 2018-11-26 12:17:16 PST
Created attachment 355666 [details]
Patch
Comment 44 Build Bot 2018-11-26 12:21:06 PST
Attachment 355666 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 266 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 45 Said Abou-Hallawa 2018-11-26 13:55:04 PST
Created attachment 355672 [details]
Patch
Comment 46 Build Bot 2018-11-26 13:59:42 PST
Attachment 355672 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 266 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 47 Said Abou-Hallawa 2018-11-26 16:34:25 PST
Created attachment 355693 [details]
Patch
Comment 48 Said Abou-Hallawa 2018-11-26 18:06:06 PST
Created attachment 355699 [details]
Patch
Comment 49 Build Bot 2018-11-26 18:12:05 PST
Attachment 355699 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 266 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 50 Build Bot 2018-11-26 21:31:24 PST
Comment on attachment 355699 [details]
Patch

Attachment 355699 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10162110

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
Comment 51 Build Bot 2018-11-26 21:31:26 PST
Created attachment 355711 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 52 Said Abou-Hallawa 2018-11-27 11:42:24 PST
Created attachment 355752 [details]
Patch
Comment 53 Build Bot 2018-11-27 11:47:52 PST
Attachment 355752 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedProperty.h:166:  Failed to find complete declaration of class PrimitiveShadow  [build/class] [5]
Total errors found: 1 in 266 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 54 Build Bot 2018-11-27 14:05:04 PST
Comment on attachment 355752 [details]
Patch

Attachment 355752 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10170799

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
svg/animations/svglength-element-removed-crash.svg
Comment 55 Build Bot 2018-11-27 14:05:18 PST
Created attachment 355777 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 56 Build Bot 2018-11-27 16:46:40 PST
Comment on attachment 355752 [details]
Patch

Attachment 355752 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10173237

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
Comment 57 Build Bot 2018-11-27 16:46:42 PST
Created attachment 355820 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 58 Said Abou-Hallawa 2018-12-02 02:18:33 PST
Created attachment 356333 [details]
Patch
Comment 59 Said Abou-Hallawa 2018-12-02 03:40:48 PST
Created attachment 356334 [details]
Patch
Comment 60 Build Bot 2018-12-02 03:45:27 PST
Attachment 356334 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 1 in 291 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 61 Build Bot 2018-12-02 08:07:22 PST
Comment on attachment 356334 [details]
Patch

Attachment 356334 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10239407

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
Comment 62 Build Bot 2018-12-02 08:07:24 PST
Created attachment 356338 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 63 Said Abou-Hallawa 2018-12-02 09:42:04 PST
Created attachment 356340 [details]
Patch
Comment 64 Build Bot 2018-12-02 09:47:50 PST
Attachment 356340 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 1 in 291 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 65 Said Abou-Hallawa 2018-12-02 11:11:45 PST
Created attachment 356342 [details]
Patch
Comment 66 Build Bot 2018-12-02 11:16:01 PST
Attachment 356342 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 1 in 291 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 67 Said Abou-Hallawa 2018-12-02 11:36:24 PST
Created attachment 356344 [details]
Patch
Comment 68 Build Bot 2018-12-02 11:40:50 PST
Attachment 356344 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 1 in 291 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 69 Build Bot 2018-12-02 13:29:08 PST
Comment on attachment 356344 [details]
Patch

Attachment 356344 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10241566

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
svg/animations/svglength-element-removed-crash.svg
Comment 70 Build Bot 2018-12-02 13:29:19 PST
Created attachment 356346 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 71 Said Abou-Hallawa 2018-12-02 17:19:24 PST
Created attachment 356351 [details]
1-SVGProperty
Comment 72 Said Abou-Hallawa 2018-12-02 18:24:03 PST
Created attachment 356352 [details]
2-SVGPathSeg
Comment 73 Said Abou-Hallawa 2018-12-02 23:31:36 PST
Created attachment 356358 [details]
3-SVGList
Comment 74 Build Bot 2018-12-02 23:59:54 PST
Comment on attachment 356344 [details]
Patch

Attachment 356344 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10245250

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
Comment 75 Build Bot 2018-12-02 23:59:57 PST
Created attachment 356360 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 76 Said Abou-Hallawa 2018-12-03 08:00:49 PST
Created attachment 356380 [details]
4-SVGDecoratedEnumeration
Comment 77 Said Abou-Hallawa 2018-12-04 07:56:12 PST
Created attachment 356497 [details]
5-SVGPathSegList
Comment 78 Said Abou-Hallawa 2018-12-04 08:59:32 PST
Created attachment 356500 [details]
6-SVGAnimatedProperty
Comment 79 Said Abou-Hallawa 2018-12-04 09:45:02 PST
Created attachment 356507 [details]
7-SVGAnimationFunction
Comment 80 Said Abou-Hallawa 2018-12-04 21:09:27 PST
Created attachment 356579 [details]
8-SVGAnimator.patch
Comment 81 Said Abou-Hallawa 2018-12-05 08:28:54 PST
Created attachment 356607 [details]
9-SVGAccessor
Comment 82 Said Abou-Hallawa 2018-12-06 08:01:50 PST
Created attachment 356728 [details]
10-SVGPropertyRegistry
Comment 83 Said Abou-Hallawa 2018-12-06 08:02:18 PST
Created attachment 356729 [details]
11-JSBinding
Comment 84 Said Abou-Hallawa 2018-12-06 08:02:55 PST
Created attachment 356730 [details]
12-FilesRemoval
Comment 85 Said Abou-Hallawa 2018-12-06 08:03:22 PST
Created attachment 356731 [details]
13-BasicShapes
Comment 86 Said Abou-Hallawa 2018-12-06 08:03:48 PST
Created attachment 356732 [details]
14-Resources
Comment 87 Said Abou-Hallawa 2018-12-06 08:04:20 PST
Created attachment 356733 [details]
15-Filters
Comment 88 Said Abou-Hallawa 2018-12-06 14:04:11 PST
Created attachment 356752 [details]
16-Text
Comment 89 Said Abou-Hallawa 2018-12-07 07:47:21 PST
Created attachment 356810 [details]
17-BaseTypes
Comment 90 Said Abou-Hallawa 2018-12-07 08:13:34 PST
Created attachment 356811 [details]
18-Conatiners
Comment 91 Said Abou-Hallawa 2018-12-08 11:33:05 PST
Created attachment 356879 [details]
19-SVGAnimateElement
Comment 92 Said Abou-Hallawa 2018-12-08 11:33:33 PST
Created attachment 356880 [details]
20-Miscellaneous
Comment 93 Said Abou-Hallawa 2018-12-08 11:34:14 PST
Created attachment 356881 [details]
21-Extra
Comment 94 Said Abou-Hallawa 2018-12-08 11:34:41 PST
Created attachment 356882 [details]
22-LayoutTests
Comment 95 Said Abou-Hallawa 2018-12-08 11:38:37 PST
Created attachment 356883 [details]
EWSPatch
Comment 96 Build Bot 2018-12-08 12:43:56 PST
Comment on attachment 356882 [details]
22-LayoutTests

Attachment 356882 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10319655

New failing tests:
svg/dom/SVGAnimatedEnumeration-SVGMarkerElement.html
svg/dom/SVGLengthList-appendItem.xhtml
svg/dom/SVGLengthList-insertItemBefore.xhtml
svg/dom/SVGPointList-basics.xhtml
svg/dom/SVGPathSegList-insertItemBefore.xhtml
svg/dom/SVGPathSegList-clear-and-initialize.xhtml
svg/dom/SVGLengthList-replaceItem.xhtml
svg/dom/SVGLengthList-basics.xhtml
svg/dom/SVGNumberList-basics.xhtml
svg/dom/SVGLengthList-removeItem.xhtml
svg/dom/SVGLengthList-initialize.xhtml
svg/dom/SVGPathSegList-appendItem.xhtml
svg/dom/SVGPathSegList-replaceItem.xhtml
svg/dom/SVGTransformList-basics.xhtml
svg/animations/svglength-element-removed-crash.svg
Comment 97 Build Bot 2018-12-08 12:43:59 PST
Created attachment 356884 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 98 Build Bot 2018-12-08 12:49:24 PST
Comment on attachment 356882 [details]
22-LayoutTests

Attachment 356882 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10319652

New failing tests:
svg/dom/SVGAnimatedEnumeration-SVGMarkerElement.html
svg/dom/SVGLengthList-appendItem.xhtml
svg/dom/SVGLengthList-insertItemBefore.xhtml
svg/dom/SVGPointList-basics.xhtml
svg/dom/SVGPathSegList-insertItemBefore.xhtml
svg/dom/SVGPathSegList-clear-and-initialize.xhtml
svg/dom/SVGLengthList-replaceItem.xhtml
svg/dom/SVGLengthList-basics.xhtml
svg/dom/SVGNumberList-basics.xhtml
svg/dom/SVGLengthList-removeItem.xhtml
svg/dom/SVGLengthList-initialize.xhtml
svg/dom/SVGPathSegList-appendItem.xhtml
svg/dom/SVGPathSegList-replaceItem.xhtml
svg/dom/SVGTransformList-basics.xhtml
svg/animations/svglength-element-removed-crash.svg
Comment 99 Build Bot 2018-12-08 12:49:27 PST
Created attachment 356885 [details]
Archive of layout-test-results from ews105 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 100 Build Bot 2018-12-08 13:28:16 PST
Comment on attachment 356882 [details]
22-LayoutTests

Attachment 356882 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10319710

New failing tests:
svg/dom/SVGAnimatedEnumeration-SVGMarkerElement.html
svg/dom/SVGLengthList-appendItem.xhtml
svg/dom/SVGLengthList-insertItemBefore.xhtml
svg/dom/SVGPointList-basics.xhtml
svg/dom/SVGPathSegList-insertItemBefore.xhtml
svg/dom/SVGPathSegList-clear-and-initialize.xhtml
svg/dom/SVGLengthList-initialize.xhtml
svg/dom/SVGNumberList-basics.xhtml
svg/dom/SVGLengthList-removeItem.xhtml
svg/dom/SVGLengthList-replaceItem.xhtml
svg/dom/SVGPathSegList-replaceItem.xhtml
svg/dom/SVGPathSegList-appendItem.xhtml
svg/dom/SVGLengthList-basics.xhtml
svg/dom/SVGTransformList-basics.xhtml
Comment 101 Build Bot 2018-12-08 13:28:19 PST
Created attachment 356887 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 102 Build Bot 2018-12-08 13:30:22 PST
Comment on attachment 356882 [details]
22-LayoutTests

Attachment 356882 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10319911

New failing tests:
svg/dom/SVGAnimatedEnumeration-SVGMarkerElement.html
svg/dom/SVGLengthList-appendItem.xhtml
svg/dom/SVGLengthList-insertItemBefore.xhtml
svg/dom/SVGPointList-basics.xhtml
svg/dom/SVGPathSegList-insertItemBefore.xhtml
svg/dom/SVGPathSegList-clear-and-initialize.xhtml
svg/dom/SVGLengthList-replaceItem.xhtml
svg/dom/SVGLengthList-basics.xhtml
svg/dom/SVGNumberList-basics.xhtml
svg/dom/SVGLengthList-removeItem.xhtml
svg/dom/SVGLengthList-initialize.xhtml
svg/dom/SVGPathSegList-appendItem.xhtml
svg/dom/SVGPathSegList-replaceItem.xhtml
svg/dom/SVGTransformList-basics.xhtml
svg/animations/svglength-element-removed-crash.svg
Comment 103 Build Bot 2018-12-08 13:30:36 PST
Created attachment 356888 [details]
Archive of layout-test-results from ews203 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews203  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 104 Build Bot 2018-12-08 13:32:45 PST
Comment on attachment 356882 [details]
22-LayoutTests

Attachment 356882 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10319702

New failing tests:
svg/dom/SVGAnimatedEnumeration-SVGMarkerElement.html
svg/dom/SVGLengthList-appendItem.xhtml
svg/dom/SVGLengthList-insertItemBefore.xhtml
svg/dom/SVGPointList-basics.xhtml
svg/dom/SVGPathSegList-insertItemBefore.xhtml
svg/dom/SVGPathSegList-clear-and-initialize.xhtml
svg/dom/SVGLengthList-replaceItem.xhtml
svg/dom/SVGLengthList-basics.xhtml
svg/dom/SVGNumberList-basics.xhtml
svg/dom/SVGLengthList-removeItem.xhtml
svg/dom/SVGLengthList-initialize.xhtml
svg/dom/SVGPathSegList-appendItem.xhtml
svg/dom/SVGPathSegList-replaceItem.xhtml
svg/dom/SVGTransformList-basics.xhtml
svg/animations/svglength-element-removed-crash.svg
Comment 105 Build Bot 2018-12-08 13:32:48 PST
Created attachment 356889 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 106 Said Abou-Hallawa 2018-12-08 16:24:38 PST
Created attachment 356892 [details]
EWSPatch
Comment 107 Build Bot 2018-12-08 16:28:38 PST
Attachment 356892 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 1 in 292 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 108 Build Bot 2018-12-08 18:17:43 PST
Comment on attachment 356892 [details]
EWSPatch

Attachment 356892 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10321663

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
svg/animations/svglength-element-removed-crash.svg
Comment 109 Build Bot 2018-12-08 18:17:56 PST
Created attachment 356895 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 110 Build Bot 2018-12-08 18:26:50 PST
Comment on attachment 356892 [details]
EWSPatch

Attachment 356892 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10321653

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
Comment 111 Build Bot 2018-12-08 18:26:53 PST
Created attachment 356896 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 112 Said Abou-Hallawa 2019-01-22 00:24:51 PST
Created attachment 359721 [details]
EWSPatch
Comment 113 Said Abou-Hallawa 2019-01-22 07:59:13 PST
Created attachment 359735 [details]
EWSPatch
Comment 114 Build Bot 2019-01-22 08:12:29 PST
Attachment 359735 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 1 in 300 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 115 Build Bot 2019-01-22 10:16:47 PST
Comment on attachment 359735 [details]
EWSPatch

Attachment 359735 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10839846

New failing tests:
imported/w3c/web-platform-tests/webrtc/simplecall.https.html
Comment 116 Build Bot 2019-01-22 10:16:50 PST
Created attachment 359747 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 117 Said Abou-Hallawa 2019-03-04 18:42:02 PST
Created attachment 363581 [details]
Rebased EWSPatch
Comment 118 Said Abou-Hallawa 2019-03-04 18:46:59 PST
*** Bug 168586 has been marked as a duplicate of this bug. ***
Comment 119 Build Bot 2019-03-04 18:49:52 PST
Attachment 363581 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/SVGURIReference.cpp:45:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 2 in 292 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 120 Build Bot 2019-03-04 20:50:22 PST
Comment on attachment 363581 [details]
Rebased EWSPatch

Attachment 363581 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11371326

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
svg/animations/svglength-element-removed-crash.svg
Comment 121 Build Bot 2019-03-04 20:50:36 PST
Created attachment 363595 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment 122 Build Bot 2019-03-05 00:36:49 PST
Comment on attachment 363581 [details]
Rebased EWSPatch

Attachment 363581 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11373978

New failing tests:
svg/dom/SVGScriptElement/script-set-href.svg
Comment 123 Build Bot 2019-03-05 00:36:52 PST
Created attachment 363613 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 124 Said Abou-Hallawa 2019-03-05 14:33:20 PST
Created attachment 363690 [details]
Patch for review
Comment 125 Build Bot 2019-03-05 14:38:38 PST
Attachment 363690 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 1 in 299 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 126 Said Abou-Hallawa 2019-03-09 23:26:15 PST
Created attachment 364172 [details]
Unified Patch
Comment 127 Build Bot 2019-03-09 23:30:59 PST
Attachment 364172 [details] did not pass style-queue:


ERROR: Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h:33:  Failed to find complete declaration of class DecoratedProperty  [build/class] [5]
Total errors found: 1 in 299 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 128 Build Bot 2019-03-10 01:26:20 PST
Comment on attachment 364172 [details]
Unified Patch

Attachment 364172 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11445830

New failing tests:
accessibility/mac/selection-notification-focus-change.html
Comment 129 Build Bot 2019-03-10 01:26:23 PST
Created attachment 364180 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 130 Darin Adler 2019-03-10 13:07:18 PDT
Comment on attachment 364172 [details]
Unified Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364172&action=review

Is there any way to make this change in a set of smaller steps? Perhaps a smaller patch that makes the behavior change, and then a bigger one that removes all the now-unnecessary code? I’m a bit overwhelmed by the total size here.

> LayoutTests/ChangeLog:15
> +        newItem from its old list before adding it to the new list. SVG2 states

"that the newItem from its old list" doesn’t make sense. Maybe "that newItem is deleted from its old list"?

> LayoutTests/ChangeLog:16
> +        that only a copy of the newItem will be inserted.

It’s not quite correct grammar to say "the newItem". I would suggest either "the new item" or "newItem" without the article.

> Source/WebCore/css/CSSCursorImageValue.cpp:108
> +    if (const auto* cursorElement = updateCursorElement(*loader.document())) {

This explicit use of const should not be needed. The auto* should match to the const type.

> Source/WebCore/rendering/svg/SVGPathData.cpp:46
> +    const auto* renderer = element.renderer();

Should not need const with auto here either. And in many other places. One of the nice things about "auto*" is that it works with both const and non-const pointers depending on what’s returned from the expression on the right.
Comment 131 Darin Adler 2019-03-10 13:08:37 PDT
Really looking forward to this change, but I am concerned with my ability to review it. Neither the smaller patches nor the unified patch seem just right to me.

But someone else may find they can review.
Comment 132 Said Abou-Hallawa 2019-03-13 18:48:21 PDT
(In reply to Darin Adler from comment #131)
> Really looking forward to this change, but I am concerned with my ability to
> review it. Neither the smaller patches nor the unified patch seem just right
> to me.
> 
> But someone else may find they can review.

I removed the SVG tear off objects for SVGAnimatedInteger in https://bugs.webkit.org/show_bug.cgi?id=195722. This the minimum of change I could get to move one property from the tear off world. Once the patch for SVGAnimatedInteger is landed, the other types will be converted similarly.
Comment 133 Nikolas Zimmermann 2019-03-20 12:57:16 PDT
Dear Said,

glad that you are tackling this. I feel guilty about the tear-off objects, since I am responsible for the mess.


Have a nice day,
Niko
Comment 134 Said Abou-Hallawa 2019-03-20 15:05:31 PDT
(In reply to Nikolas Zimmermann from comment #133)
> Dear Said,
> 
> glad that you are tackling this. I feel guilty about the tear-off objects,
> since I am responsible for the mess.
> 
> 
> Have a nice day,
> Niko

Hi Niko,

Actually it was an honor for me to work in the code that you wrote. I learnt a lot from it. And I understand a big part of it was written at the time the septs was being changed. I was lucky to find a working code and finalized specs.

I wish you will have some time to write some patches for SVG 2 and review my patches also.

Thanks for dropping this message.

Said
Comment 135 Nikolas Zimmermann 2019-03-24 02:05:28 PDT
(In reply to Said Abou-Hallawa from comment #134)
> (In reply to Nikolas Zimmermann from comment #133)
> Actually it was an honor for me to work in the code that you wrote. I learnt
> a lot from it. And I understand a big part of it was written at the time the
> septs was being changed. I was lucky to find a working code and finalized
> specs.
Oh yes, the SVG DOM turned out to be difficult to implement, and some parts were really obscure (SVG DOM live list synchronization, with attributes comes to my mind, etc.). Glad that you are working on it.

> 
> I wish you will have some time to write some patches for SVG 2 and review my
> patches also.
I am no longer a reviewer - even if I am technically, I would not feel comfortable with reviewing after my long absence. I've followed each WebKit commit since I left in early 2013, but I am no longer actively working on it - busy with finishing my PhD at present. I hope to find time again for SVG once I am done with my journey in physics...

Have a nice day,
Niko
Comment 136 Said Abou-Hallawa 2019-04-02 11:36:09 PDT
Created attachment 366512 [details]
Patch
Comment 137 Said Abou-Hallawa 2019-04-02 17:18:11 PDT
Created attachment 366557 [details]
Patch
Comment 138 Simon Fraser (smfr) 2019-04-03 11:45:04 PDT
Comment on attachment 366557 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366557&action=review

> Source/WebCore/svg/SVGAnimateElementBase.cpp:97
> +    m_hasInvalidCSSAttributeType = WTF::nullopt;

= { }

> Source/WebCore/svg/SVGAnimateElementBase.cpp:154
> +void SVGAnimateElementBase::calculateAnimatedValue(float percentage, unsigned repeatCount, SVGSMILElement*)

"percentage" is the wrong term here, because it's 0-1, not 0-100. We usually call this "progress"

> Source/WebCore/svg/SVGAnimateElementBase.cpp:192
> +        return -1;

Better to return an Optional<float>

> Source/WebCore/svg/SVGElement.cpp:567
> +void SVGElement::synchronizeAllAnimatedSVGAttribute(SVGElement* svgElement)

Can we pass SVGElement&?

> Source/WebCore/svg/SVGViewElement.h:48
> +    // FIXME: svgAttributeChanged missing.

File a bug for this? It's not clear what the implications are.
Comment 139 Said Abou-Hallawa 2019-04-03 13:43:10 PDT
Created attachment 366636 [details]
Patch
Comment 140 Said Abou-Hallawa 2019-04-03 13:52:44 PDT
Comment on attachment 366557 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366557&action=review

>> Source/WebCore/svg/SVGAnimateElementBase.cpp:97
>> +    m_hasInvalidCSSAttributeType = WTF::nullopt;
> 
> = { }

Fixed.

>> Source/WebCore/svg/SVGAnimateElementBase.cpp:154
>> +void SVGAnimateElementBase::calculateAnimatedValue(float percentage, unsigned repeatCount, SVGSMILElement*)
> 
> "percentage" is the wrong term here, because it's 0-1, not 0-100. We usually call this "progress"

Done.

But I had to rename the 'progress()' methods of the SVGAttributeAnimator and the SVGAnimationFunction to 'animate()'. So I can rename their parameter 'percentage' to 'process'

>> Source/WebCore/svg/SVGAnimateElementBase.cpp:192
>> +        return -1;
> 
> Better to return an Optional<float>

Done.

But I had to change the base class and all the other driven classes as well. And I had also to change the return type of SVGAttributeAnimator::calculateDistance().

>> Source/WebCore/svg/SVGElement.cpp:567
>> +void SVGElement::synchronizeAllAnimatedSVGAttribute(SVGElement* svgElement)
> 
> Can we pass SVGElement&?

Done.

This function is only referenced from SelectorCodeGenerator::generateSynchronizeAllAnimatedSVGAttribute(). It looks like it gets its addresses but I do not know if it is really called or not.

>> Source/WebCore/svg/SVGViewElement.h:48
>> +    // FIXME: svgAttributeChanged missing.
> 
> File a bug for this? It's not clear what the implications are.

Filed https://bugs.webkit.org/show_bug.cgi?id=196554. And I changed the comment above to include the bug number.
Comment 141 WebKit Commit Bot 2019-04-03 15:08:06 PDT
Comment on attachment 366636 [details]
Patch

Clearing flags on attachment: 366636

Committed r243830: <https://trac.webkit.org/changeset/243830>
Comment 142 WebKit Commit Bot 2019-04-03 15:08:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 143 Said Abou-Hallawa 2019-04-13 02:26:16 PDT
*** Bug 191372 has been marked as a duplicate of this bug. ***
Comment 144 Said Abou-Hallawa 2019-05-06 12:40:15 PDT
*** Bug 135040 has been marked as a duplicate of this bug. ***
Comment 145 Said Abou-Hallawa 2019-05-06 14:57:33 PDT
*** Bug 132557 has been marked as a duplicate of this bug. ***
Comment 146 Said Abou-Hallawa 2019-05-06 15:20:26 PDT
*** Bug 91191 has been marked as a duplicate of this bug. ***
Comment 147 Said Abou-Hallawa 2019-05-06 15:28:26 PDT
*** Bug 184670 has been marked as a duplicate of this bug. ***