Bug 39091 - Rename WebGLArray types to TypedArray types
Summary: Rename WebGLArray types to TypedArray types
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kenneth Russell
URL:
Keywords:
Depends on: 39036
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-13 15:32 PDT by Kenneth Russell
Modified: 2011-01-12 01:23 PST (History)
15 users (show)

See Also:


Attachments
Patch (48.79 KB, patch)
2010-05-13 17:37 PDT, Kenneth Russell
dglazkov: review-
kbr: commit-queue-
Details | Formatted Diff | Diff
Revised patch (49.40 KB, patch)
2010-05-14 14:51 PDT, Kenneth Russell
darin: review+
kbr: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 2010-05-13 15:32:49 PDT
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.
Comment 1 Kenneth Russell 2010-05-13 17:37:39 PDT
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.
Comment 2 WebKit Review Bot 2010-05-13 17:42:53 PDT
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.
Comment 3 Early Warning System Bot 2010-05-13 17:48:54 PDT
Attachment 56036 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/2283034
Comment 4 Kenneth Russell 2010-05-13 17:51:22 PDT
(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 .
Comment 5 WebKit Review Bot 2010-05-13 18:02:19 PDT
Attachment 56036 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2248056
Comment 6 Kenneth Russell 2010-05-13 18:02:46 PDT
(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?
Comment 7 WebKit Review Bot 2010-05-13 18:05:42 PDT
Attachment 56036 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/2296029
Comment 8 Adam Barth 2010-05-13 18:10:14 PDT
> 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.  :)
Comment 9 Kenneth Russell 2010-05-13 18:12:29 PDT
(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.
Comment 10 Dimitri Glazkov (Google) 2010-05-14 12:49:45 PDT
Comment on attachment 56036 [details]
Patch

rs=me.
Comment 11 Dimitri Glazkov (Google) 2010-05-14 12:54:50 PDT
Comment on attachment 56036 [details]
Patch

Per our IM conversation: "maybe i should guard the more permissive file matching regexp with whether isDOMTypeRename == 1" :)
Comment 12 Kenneth Russell 2010-05-14 14:51:50 PDT
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.
Comment 13 WebKit Review Bot 2010-05-14 14:53:16 PDT
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.
Comment 14 Early Warning System Bot 2010-05-14 15:01:44 PDT
Attachment 56113 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/2250096
Comment 15 Darin Adler 2010-05-14 15:24:31 PDT
Comment on attachment 56113 [details]
Revised patch

Looks OK. r=me
Comment 16 Darin Adler 2010-05-14 15:25:12 PDT
But it looks like the builds are failing.
Comment 17 Kenneth Russell 2010-05-14 15:34:54 PDT
(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.
Comment 18 Kenneth Russell 2010-05-14 15:37:29 PDT
Committed r59499: <http://trac.webkit.org/changeset/59499>