http://codereview.chromium.org/159263/diff/55/58 File include/v8.h (right):
http://codereview.chromium.org/159263/diff/55/58#newcode1172 Line 1172: void SetElementsToPixelData(uint8_t* data, int length); On 2009/07/28 06:07:27, Kasper Lund wrote: > I still think you should avoid using the Elements name here, because it's not > really a term we use in the API. Also a comment on what this does and that the > 'embedder' still owns the data would be nice. Changed to SetIndexedPropertiesToPixelData to be inline with other naming within this file. http://codereview.chromium.org/159263/diff/55/59 File src/api.cc (right): http://codereview.chromium.org/159263/diff/55/59#newcode2201 Line 2201: i::Handle<i::PixelArray> pixels = i::Factory::NewPixelArray(length, data); On 2009/07/28 06:07:27, Kasper Lund wrote: > Should we have an API check (that also checks in release mode) that self isn't a > JSArray? I think that would be nice given the constraints of the current > implementation. Maybe also check that length is a valid smi? Done. http://codereview.chromium.org/159263/diff/55/65 File src/api.h (right): http://codereview.chromium.org/159263/diff/55/65#newcode102 Line 102: ASSERT(value()->GetElementsKind() == On 2009/07/28 06:07:27, Kasper Lund wrote: > Keep as HasFastElements? Done. http://codereview.chromium.org/159263/diff/55/65#newcode109 Line 109: ASSERT(value_->GetElementsKind() == On 2009/07/28 06:07:27, Kasper Lund wrote: > Keep as HasFastElements? Done. http://codereview.chromium.org/159263/diff/55/64 File src/ia32/ic-ia32.cc (right): http://codereview.chromium.org/159263/diff/55/64#newcode287 Line 287: // Check whether the elements are a pixel array. On 2009/07/28 06:07:27, Kasper Lund wrote: > are -> is Done. http://codereview.chromium.org/159263/diff/55/62 File src/objects.cc (right): http://codereview.chromium.org/159263/diff/55/62#newcode1676 Line 1676: switch (JSObject::cast(pt)->GetElementsKind()) { On 2009/07/28 06:07:27, Kasper Lund wrote: > It might be clearer with !JSObject::cast(pt)->HasDictionaryElements, but I'm > okay either way. Done. http://codereview.chromium.org/159263/diff/55/62#newcode5879 Line 5879: case PIXEL_ELEMENTS: { On 2009/07/28 06:07:27, Kasper Lund wrote: > Move above the DICTIONARY_ELEMENTS case to match the more common ordering. Done. http://codereview.chromium.org/159263/diff/55/67 File src/objects.h (right): http://codereview.chromium.org/159263/diff/55/67#newcode115 Line 115: class Array; On 2009/07/28 06:07:27, Kasper Lund wrote: > Move this to globals.h? Done. http://codereview.chromium.org/159263/diff/55/67#newcode2472 Line 2472: // This accessor applies the correct conversion and clamping on the value. On 2009/07/28 06:07:27, Kasper Lund wrote: > Maybe extend the comment to state that it only deals with numbers and undefined > values? Done. http://codereview.chromium.org/159263 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
