Fixes a crash when entering a negative index with chrome.tabs.move ....

Fixes a bug where the json_schema.js would ignore 'minimum' and 'maximum' when the value of either was 0.
Also getType() was returning "integer" for values outside the range of an integer.  getType() now returns "number" for large values.

Unit tests are also added to spot these problems.

Follow up to 
ISSUE=5029001
BUG=53990

[Aaron Boodman, 2010-11-16 20:21:28]

Great, almost there.

For future reference, you don't need to create a new issue on this site for each
change. You can just do 'gcl upload' again, and it will update this page with
your new patch.

This is useful so that your reviewer can see what you changed.

http://codereview.chromium.org/5018002/diff/1/chrome/test/data/extensions/api...
File chrome/test/data/extensions/api_test/tabs/basics/move.html (right):

http://codereview.chromium.org/5018002/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/tabs/basics/move.html:73: // Check that the
browser doesn't segfault on a negative index
Nit: add a period to the end of this sentence. Also, add a reference to the bug
here that we fixed so that people can see the background.

http://codereview.chromium.org/5018002/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/tabs/basics/move.html:76: try{
Nit: spaces before "{":
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Conditionals

http://codereview.chromium.org/5018002/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/tabs/basics/move.html:80: }
Add a comment inside the catch block like:

// No need to test details of error handling here; they 
// should be tested by JSON schema tests.

http://codereview.chromium.org/5018002/diff/1/chrome/test/data/extensions/jso...
File chrome/test/data/extensions/json_schema_test.js (right):

http://codereview.chromium.org/5018002/diff/1/chrome/test/data/extensions/jso...
chrome/test/data/extensions/json_schema_test.js:423: //0 casts to bool
differently, so it's important to test
Space after "//". Add a period to the end of the comment. And again, add a
reference to the bug.

[Aaron Boodman, 2010-11-18 23:04:35]

LGTM w/ the below exception.

Don't worry about a new patch. I will do this when I land it.

Thanks for the contribution!

http://codereview.chromium.org/5018002/diff/26001/chrome/renderer/resources/j...
File chrome/renderer/resources/json_schema.js (right):

http://codereview.chromium.org/5018002/diff/26001/chrome/renderer/resources/j...
chrome/renderer/resources/json_schema.js:108: var maxInt = -1 >>> 1;
This is not the largest representable integer in JavaScript. That honor goes to
Math.pow(2, 53). For background on why this is, see:
http://en.wikipedia.org/wiki/Double_precision

[cheesedanish, 2010-11-19 03:51:01]

A posible correction to his statement.  The C code expects an integer to be an
int32.  Make sure it works with values greater than 2^31.
-W
------Original Message------
From: aa@chromium.org
To: waltermil@gmail.com
Cc: chromium-reviews@chromium.org
Cc: erikkay@chromium.org
Cc: brettw-cc@chromium.org
Cc: pam+watch@chromium.org
Cc: phajdan.jr@chromium.org
Cc: darin-cc@chromium.org
ReplyTo: waltermil@gmail.com
ReplyTo: aa@chromium.org
ReplyTo: chromium-reviews@chromium.org
ReplyTo: erikkay@chromium.org
ReplyTo: brettw-cc@chromium.org
ReplyTo: pam+watch@chromium.org
ReplyTo: phajdan.jr@chromium.org
ReplyTo: darin-cc@chromium.org
Subject: Re: Fixes a crash when entering a negative index withchrome.tabs.move
.... (issue5018002)
Sent: Nov 18, 2010 6:04 PM

LGTM w/ the below exception.

Don't worry about a new patch. I will do this when I land it.

Thanks for the contribution!


http://codereview.chromium.org/5018002/diff/26001/chrome/renderer/resources/j...
File chrome/renderer/resources/json_schema.js (right):

http://codereview.chromium.org/5018002/diff/26001/chrome/renderer/resources/j...
chrome/renderer/resources/json_schema.js:108: var maxInt = -1 >>> 1;
This is not the largest representable integer in JavaScript. That honor
goes to Math.pow(2, 53). For background on why this is, see:
http://en.wikipedia.org/wiki/Double_precision

http://codereview.chromium.org/5018002/


Sent from my “contract free” BlackBerry® smartphone on the WIND network.