Bug 39091 – Rename WebGLArray types to TypedArray types

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Kenneth Russell]

The WebGL specification is splitting off the WebGL<T>Array types into a separate TypedArray specification. The WebKit WebGL implementation needs to rename these classes to track this change.

[Kenneth Russell]

Created attachment 56036 [details]
Patch

Note: this patch only contains those few files, including do-webcore-rename, which had to be modified by hand. The full patch containing the results of running do-webcore-rename is over 500k.

From the ChangeLog:

Extended functionality of do-webcore-rename script and used it to rename the WebGLArray types to the TypedArray naming convention. The only source files which were touched by hand, and which are being manually reviewed, are:
    WebCore/page/DOMWindow.idl
    WebCore/bindings/generic/RuntimeEnabledFeatures.h (script's changes undone)
    WebKit/WebCore/bindings/js/JSDOMWindowCustom.cpp
    WebKit/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
These only needed to be touched to update the aliases between the WebGLArray and TypedArray names introduced in bug 39036. (It was not feasible to have do-webcore-rename handle this as it would introduce circular renamings.) These aliases will be removed in roughly a month once existing WebGL content has been updated.

No new tests; covered under existing WebGL tests. Updated constructed-objects-prototypes and prototype-inheritance-2 tests. Ran all layout tests in Safari and all WebGL tests in Chromium.

[WebKit Review Bot]

Attachment 56036 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:75:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:77:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:79:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:80:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/js/JSDOMWindowCustom.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/js/JSDOMWindowCustom.cpp:66:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/js/JSDOMWindowCustom.cpp:68:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 7 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Attachment 56036 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/2283034

[Kenneth Russell]

(In reply to comment #2)
> Attachment 56036 [details] did not pass style-queue:
> 
> Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
> WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:75:  Alphabetical sorting problem.  [build/include_order] [4]
> WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:77:  Alphabetical sorting problem.  [build/include_order] [4]
> WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:79:  Alphabetical sorting problem.  [build/include_order] [4]
> WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:80:  Alphabetical sorting problem.  [build/include_order] [4]
> WebCore/bindings/js/JSDOMWindowCustom.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
> WebCore/bindings/js/JSDOMWindowCustom.cpp:66:  Alphabetical sorting problem.  [build/include_order] [4]
> WebCore/bindings/js/JSDOMWindowCustom.cpp:68:  Alphabetical sorting problem.  [build/include_order] [4]
> Total errors found: 7 in 10 files

Note: these style cleanups, which are caused by running the do-webcore-rename script, will be addressed separately under https://bugs.webkit.org/show_bug.cgi?id=38761 .

[WebKit Review Bot]

Attachment 56036 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2248056

[Kenneth Russell]

(In reply to comment #3)
> Attachment 56036 [details] did not build on qt:
> Build output: http://webkit-commit-queue.appspot.com/results/2283034

I should have realized that this patch wouldn't build on any platforms. Would it be better if I uploaded a full version not for the reviewers, but for the bots to try?

[WebKit Review Bot]

Attachment 56036 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/2296029

[Adam Barth]

> I should have realized that this patch wouldn't build on any platforms. Would it be better if I uploaded a full version not for the reviewers, but for the bots to try?

I wouldn't worry too much about it.  You just want to be sure you're not going to land something that doesn't compile.  :)

[Kenneth Russell]

(In reply to comment #8)
> > I should have realized that this patch wouldn't build on any platforms. Would it be better if I uploaded a full version not for the reviewers, but for the bots to try?
> 
> I wouldn't worry too much about it.  You just want to be sure you're not going to land something that doesn't compile.  :)

I can guarantee it compiles in WebKit/Mac and Chromium/Mac but didn't try any other OSs or WebKit ports.

[Dimitri Glazkov (Google)]

Comment on attachment 56036 [details]
Patch

rs=me.

[Dimitri Glazkov (Google)]

Comment on attachment 56036 [details]
Patch

Per our IM conversation: "maybe i should guard the more permissive file matching regexp with whether isDOMTypeRename == 1" :)

[Kenneth Russell]

Created attachment 56113 [details]
Revised patch

Changed do-webcore-rename so that it only uses the more permissive regexp matching of the file name if it's doing a DOM type rename. This will avoid accidentally renaming files if the string being renamed is a common substring of file names.

[WebKit Review Bot]

Attachment 56113 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:75:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:77:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:79:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:80:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/js/JSDOMWindowCustom.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/js/JSDOMWindowCustom.cpp:66:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/js/JSDOMWindowCustom.cpp:68:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 7 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Attachment 56113 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/2250096

[Darin Adler]

Comment on attachment 56113 [details]
Revised patch

Looks OK. r=me

[Darin Adler]

But it looks like the builds are failing.

[Kenneth Russell]

(In reply to comment #16)
> But it looks like the builds are failing.

Right. That's because these are only a tiny portion of the overall changes, the rest of which are done by do-webcore-rename. I'll do the commit and watch the bots to see if any platforms I haven't tested break, and roll it out if so.

[Kenneth Russell]

Committed r59499: <http://trac.webkit.org/changeset/59499>