From 27211db0ccc9c3ff7b32a5cfd310288418656fc2 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Thu, 21 Apr 2011 18:59:30 -0400 Subject: [PATCH] Fixed: JS objects 'randomly' failed to unarchive. The problem was that if the same JS object was decoded twice as part of a more complex object, the first decoded version was cached in its unwrapped state and not properly unwrapped the second time. This caused very hard to debug theming errors where e.g. a CPTextField's custom inset would decode fine, but a subclass of a CPTextField otherwise exactly the same would fail to decode the inset. --- Foundation/CPKeyedUnarchiver.j | 135 +++++++++++++------------ Tests/Foundation/CPKeyedArchiverTest.j | 44 ++++++++ 2 files changed, 113 insertions(+), 66 deletions(-) create mode 100644 Tests/Foundation/CPKeyedArchiverTest.j diff --git a/Foundation/CPKeyedUnarchiver.j b/Foundation/CPKeyedUnarchiver.j index 666b4f1e3..b9baf93c5 100644 --- a/Foundation/CPKeyedUnarchiver.j +++ b/Foundation/CPKeyedUnarchiver.j @@ -441,71 +441,48 @@ var _CPKeyedUnarchiverDecodeObjectAtIndex = function(self, anIndex) var object = self._objects[anIndex]; if (object) + { if (object === self._objects[0]) return nil; - else - return object; - - var object, - plistObject = self._plistObjects[anIndex], - plistObjectClass = plistObject.isa; - - if (plistObjectClass === CPDictionaryClass || plistObjectClass === CPMutableDictionaryClass) + // Don't return immediately here. The _CPKeyedArchiverValueClass unwrapper code + // hasn't executed yet. + } + else { - var plistClass = self._plistObjects[plistObject.valueForKey(_CPKeyedArchiverClassKey).valueForKey(_CPKeyedArchiverUIDKey)], - className = plistClass.valueForKey(_CPKeyedArchiverClassNameKey), - classes = plistClass.valueForKey(_CPKeyedArchiverClassesKey), - theClass = [self classForClassName:className]; + var plistObject = self._plistObjects[anIndex], + plistObjectClass = plistObject.isa; - if (!theClass) - theClass = CPClassFromString(className); - - if (!theClass && (self._delegateSelectors & CPKeyedUnarchiverDelegate_unarchiver_cannotDecodeObjectOfClassName_originalClasses_)) - theClass = [_delegate unarchiver:self cannotDecodeObjectOfClassName:className originalClasses:classes]; - - if (!theClass) - [CPException raise:CPInvalidUnarchiveOperationException reason:@"-[CPKeyedUnarchiver decodeObjectForKey:]: cannot decode object of class (" + className + @")"]; - - var savedPlistObject = self._plistObject; - - self._plistObject = plistObject; - - // Should we only call this on _CPCibClassSwapper? (currently the only class that makes use of this). - object = [theClass allocWithCoder:self]; - - // It is important to do this before calling initWithCoder so that decoding can be self referential (something = self). - self._objects[anIndex] = object; - - var processedObject = [object initWithCoder:self]; - - self._plistObject = savedPlistObject; - - if (processedObject !== object) + if (plistObjectClass === CPDictionaryClass || plistObjectClass === CPMutableDictionaryClass) { - if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) - [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; + var plistClass = self._plistObjects[plistObject.valueForKey(_CPKeyedArchiverClassKey).valueForKey(_CPKeyedArchiverUIDKey)], + className = plistClass.valueForKey(_CPKeyedArchiverClassNameKey), + classes = plistClass.valueForKey(_CPKeyedArchiverClassesKey), + theClass = [self classForClassName:className]; - object = processedObject; - self._objects[anIndex] = processedObject; - } + if (!theClass) + theClass = CPClassFromString(className); - processedObject = [object awakeAfterUsingCoder:self]; + if (!theClass && (self._delegateSelectors & CPKeyedUnarchiverDelegate_unarchiver_cannotDecodeObjectOfClassName_originalClasses_)) + theClass = [_delegate unarchiver:self cannotDecodeObjectOfClassName:className originalClasses:classes]; - if (processedObject !== object) - { - if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) - [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; + if (!theClass) + [CPException raise:CPInvalidUnarchiveOperationException reason:@"-[CPKeyedUnarchiver decodeObjectForKey:]: cannot decode object of class (" + className + @")"]; - object = processedObject; - self._objects[anIndex] = processedObject; - } + var savedPlistObject = self._plistObject; - if (self._delegate) - { - if (self._delegateSelectors & _CPKeyedUnarchiverDidDecodeObjectSelector) - processedObject = [self._delegate unarchiver:self didDecodeObject:object]; + self._plistObject = plistObject; - if (processedObject && processedObject != object) + // Should we only call this on _CPCibClassSwapper? (currently the only class that makes use of this). + object = [theClass allocWithCoder:self]; + + // It is important to do this before calling initWithCoder so that decoding can be self referential (something = self). + self._objects[anIndex] = object; + + var processedObject = [object initWithCoder:self]; + + self._plistObject = savedPlistObject; + + if (processedObject !== object) { if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; @@ -513,22 +490,48 @@ var _CPKeyedUnarchiverDecodeObjectAtIndex = function(self, anIndex) object = processedObject; self._objects[anIndex] = processedObject; } - } - } - else - { - self._objects[anIndex] = object = plistObject; - if ([object class] === CPStringClass) - { - if (object === _CPKeyedArchiverNullString) + processedObject = [object awakeAfterUsingCoder:self]; + + if (processedObject !== object) { - self._objects[anIndex] = self._objects[0]; + if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) + [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; - return nil; + object = processedObject; + self._objects[anIndex] = processedObject; + } + + if (self._delegate) + { + if (self._delegateSelectors & _CPKeyedUnarchiverDidDecodeObjectSelector) + processedObject = [self._delegate unarchiver:self didDecodeObject:object]; + + if (processedObject && processedObject != object) + { + if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) + [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; + + object = processedObject; + self._objects[anIndex] = processedObject; + } + } + } + else + { + self._objects[anIndex] = object = plistObject; + + if ([object class] === CPStringClass) + { + if (object === _CPKeyedArchiverNullString) + { + self._objects[anIndex] = self._objects[0]; + + return nil; + } + else + self._objects[anIndex] = object = plistObject; } - else - self._objects[anIndex] = object = plistObject; } } diff --git a/Tests/Foundation/CPKeyedArchiverTest.j b/Tests/Foundation/CPKeyedArchiverTest.j new file mode 100644 index 000000000..ea61e832e --- /dev/null +++ b/Tests/Foundation/CPKeyedArchiverTest.j @@ -0,0 +1,44 @@ +@import +@import + +@implementation CPKeyedArchiverTest : OJTestCase +{ + +} + +- (void)testJavaScriptObject +{ + var original = [Archivable new]; + + [original setAJsObject:{ 'top': 5 }]; + + var decoded = [CPKeyedUnarchiver unarchiveObjectWithData:[CPKeyedArchiver archivedDataWithRootObject:original]] + + [self assert:5 equals:[decoded aJsObject].top message:"JS object encoded and decoded right"]; +} + +@end + +@implementation Archivable : CPObject +{ + JSObject aJsObject @accessors; +} + +- (id)initWithCoder:(CPCoder)aCoder +{ + if (self = [super init]) + { + // Note we decode this twice to expose a bug where the cached decoded + // values did not properly unwrap JS objects. + aJsObject = [aCoder decodeObjectForKey:@"aJsObject"]; + aJsObject = [aCoder decodeObjectForKey:@"aJsObject"]; + } + return self; +} + +- (void)encodeWithCoder:(CPCoder)aCoder +{ + [aCoder encodeObject:aJsObject forKey:@"aJsObject"]; +} + +@end