[Webkit-unassigned] [Bug 58280] SerializedScriptValue shouldn't always throw exceptions

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 11 16:16:15 PDT 2011


https://bugs.webkit.org/show_bug.cgi?id=58280


Oliver Hunt <oliver at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89113|review?                     |review-
               Flag|                            |




--- Comment #3 from Oliver Hunt <oliver at apple.com>  2011-04-11 16:16:15 PST ---
(From update of attachment 89113)
View in context: https://bugs.webkit.org/attachment.cgi?id=89113&action=review

Rather than a bool i'd prefer
enum SerializationErrorMode { Throwing, NonThrowing }

as that makes things more explicit.

The use of Global will also fail on ToT as geoff has renamed everything, i believe you'll want heap/Strong.h and Strong<..>

r-ing due to the style errors, and it failing to build on tot :-(

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:1435
> +    switch (code) {
> +    case SuccessfullyCompleted:
> +    	break;
> +    case StackOverflowError:
> +    	throwError(exec, createStackOverflowError(exec));
> +    	break;
> +    case InterruptedExecutionError:
> +    	throwError(exec, createInterruptedExecutionException(&exec->globalData()));
> +    	break;
> +    case ValidationError:
> +    	throwError(exec, createTypeError(exec, "Unable to deserialize data."));
> +    	break;
> +    case ExistingExceptionError:
> +    case UnspecifiedError:        
> +    	break;
> +    }

I don't like this code, i'd prefer:
if (code == SuccessfullyCompleted)
    return;
switch (code) {
case ...:
   throwError(exec, ...)
   return;
default:
   return;

Also UnspecifiedError and ExistingExceptionError should both create a generic exception message (as we apparently don't know what has gone wrong).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list