Bug 92181 - Replace int with long in test IDL files
Summary: Replace int with long in test IDL files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-24 17:25 PDT by Kentaro Hara
Modified: 2012-07-25 10:05 PDT (History)
8 users (show)

See Also:


Attachments
Patch (67.30 KB, patch)
2012-07-24 17:27 PDT, Kentaro Hara
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-01 (1.02 MB, application/zip)
2012-07-24 18:31 PDT, WebKit Review Bot
no flags Details
Archive of layout-test-results from gce-cq-03 (1013.29 KB, application/zip)
2012-07-24 18:51 PDT, WebKit Review Bot
no flags Details
patch for landing (67.29 KB, patch)
2012-07-25 02:24 PDT, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2012-07-24 17:25:53 PDT
int is not a valid type in WebIDL. It should be long. We should replace int with long in bindings/scripts/test/*.idl.

This is a follow-up patch for r123550.
Comment 1 Kentaro Hara 2012-07-24 17:27:53 PDT
Created attachment 154189 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-24 18:30:43 PDT
Comment on attachment 154189 [details]
Patch

Attachment 154189 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13347052

New failing tests:
http/tests/security/script-crossorigin-loads-correctly.html
fast/loader/loadInProgress.html
fast/inline/positionedLifetime.html
fast/loader/unload-form-post-about-blank.html
Comment 3 WebKit Review Bot 2012-07-24 18:31:06 PDT
Created attachment 154210 [details]
Archive of layout-test-results from gce-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 4 WebKit Review Bot 2012-07-24 18:51:51 PDT
Comment on attachment 154189 [details]
Patch

Rejecting attachment 154189 [details] from commit-queue.

New failing tests:
http/tests/security/script-crossorigin-loads-correctly.html
fast/loader/loadInProgress.html
fast/inline/positionedLifetime.html
fast/loader/unload-form-post-about-blank.html
Full output: http://queues.webkit.org/results/13343058
Comment 5 WebKit Review Bot 2012-07-24 18:51:56 PDT
Created attachment 154216 [details]
Archive of layout-test-results from gce-cq-03

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: gce-cq-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 6 Adam Barth 2012-07-24 19:20:09 PDT
That makes no sense!
Comment 7 Kentaro Hara 2012-07-24 19:21:11 PDT
Yeah, let me cq+ it later again.
Comment 8 Kentaro Hara 2012-07-25 02:24:19 PDT
Created attachment 154302 [details]
patch for landing
Comment 9 WebKit Review Bot 2012-07-25 03:35:49 PDT
Comment on attachment 154302 [details]
patch for landing

Clearing flags on attachment: 154302

Committed r123600: <http://trac.webkit.org/changeset/123600>
Comment 10 Vineet Chaudhary (vineetc) 2012-07-25 06:18:53 PDT
(In reply to comment #0)
> int is not a valid type in WebIDL. It should be long. We should replace int with long in bindings/scripts/test/*.idl.
> 
> This is a follow-up patch for r123550.

haraken, sorry if I am getting this wrong but does this mean we should not use int WebCore/*.idl files?
like one is 
.../dom/Touch.idl: readonly attribute int webkitRadiusX;
Comment 11 Kentaro Hara 2012-07-25 07:34:48 PDT
(In reply to comment #10)
> haraken, sorry if I am getting this wrong but does this mean we should not use int WebCore/*.idl files?
> like one is 
> .../dom/Touch.idl: readonly attribute int webkitRadiusX;

Per the spec, int is not allowed. But we need to care about compatibility when we replace the real IDL files. (Sorry I've not yet looked into them in detail.)
Comment 12 Adam Barth 2012-07-25 09:00:12 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > haraken, sorry if I am getting this wrong but does this mean we should not use int WebCore/*.idl files?
> > like one is 
> > .../dom/Touch.idl: readonly attribute int webkitRadiusX;
> 
> Per the spec, int is not allowed. But we need to care about compatibility when we replace the real IDL files. (Sorry I've not yet looked into them in detail.)

There shouldn't be any compat trouble since they're both JavaScript numbers.
Comment 13 Vineet Chaudhary (vineetc) 2012-07-25 09:28:00 PDT
(In reply to comment #12)

> There shouldn't be any compat trouble since they're both JavaScript numbers.

Thanks haraken/abarth for clarifying currently there are below listed idls with int attributes

./dom/Touch.idl:        readonly attribute int              webkitRadiusX;
./dom/Touch.idl:        readonly attribute int              webkitRadiusY;
./dom/WebKitNamedFlow.idl:        readonly attribute int firstEmptyRegionIndex;
./html/canvas/WebGLActiveInfo.idl:        readonly attribute int size;
./html/canvas/WebGLActiveInfo.idl:        readonly attribute unsigned int type;
./html/canvas/WebGLShaderPrecisionFormat.idl:        readonly attribute int rangeMin;
./html/canvas/WebGLShaderPrecisionFormat.idl:        readonly attribute int rangeMax;
./html/canvas/WebGLShaderPrecisionFormat.idl:        readonly attribute int precision;
./page/WebKitAnimation.idl:        readonly attribute [Custom] int iterationCount;

If its oke to replace these with long I can provide patch for all of them in one or separate patch.
For ArrayBuffer.idl I have filed Bug92260 already.
Comment 14 Adam Barth 2012-07-25 10:05:45 PDT
I'd do them all together.