Bug 93792

Summary: [V8] Factor out exception related methods of V8Proxy
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: WebCore JavaScriptAssignee: Kentaro Hara <haraken>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, a.renevier, dglazkov, eric.carlson, feature-media-reviews, gyuyoung.kim, japhet, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 85330    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from gce-cr-linux-06
none
Patch none

Description Kentaro Hara 2012-08-12 21:36:05 PDT
To remove V8Proxy, we can factor out exception related methods of V8Proxy to a separate file.
Comment 1 Kentaro Hara 2012-08-12 22:24:28 PDT
Created attachment 157919 [details]
Patch
Comment 2 WebKit Review Bot 2012-08-12 23:24:17 PDT
Comment on attachment 157919 [details]
Patch

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

New failing tests:
http/tests/security/document-all.html
Comment 3 WebKit Review Bot 2012-08-12 23:24:23 PDT
Created attachment 157924 [details]
Archive of layout-test-results from gce-cr-linux-06

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 4 Kentaro Hara 2012-08-13 01:13:09 PDT
Created attachment 157937 [details]
Patch
Comment 5 Adam Barth 2012-08-13 10:24:03 PDT
Comment on attachment 157937 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=157937&action=review

> Source/WebCore/bindings/v8/V8ThrowException.cpp:34
> +static v8::Handle<v8::Value> DOMExceptionStackGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)

DOMExceptionStackGetter -> domExceptionStackGetter

> Source/WebCore/bindings/v8/V8ThrowException.cpp:40
> +static void DOMExceptionStackSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)

DOMExceptionStackSetter -> domExceptionStackSetter

> Source/WebCore/bindings/v8/V8ThrowException.cpp:58
> +        const char* message = 0;
> +        return V8ThrowException::throwTypeError(message, isolate);

I would have combined these lines.

> Source/WebCore/bindings/v8/V8ThrowException.h:32
> +// The types of javascript errors that can be thrown.

This comment seems useless and probably should be removed.
Comment 6 Kentaro Hara 2012-08-13 19:04:47 PDT
Committed r125495: <http://trac.webkit.org/changeset/125495>
Comment 7 Kentaro Hara 2012-08-13 19:09:08 PDT
(In reply to comment #5)
> > Source/WebCore/bindings/v8/V8ThrowException.cpp:34
> > +static v8::Handle<v8::Value> DOMExceptionStackGetter(v8::Local<v8::String> name, const v8::AccessorInfo& info)
> 
> DOMExceptionStackGetter -> domExceptionStackGetter
> 
> > Source/WebCore/bindings/v8/V8ThrowException.cpp:40
> > +static void DOMExceptionStackSetter(v8::Local<v8::String> name, v8::Local<v8::Value> value, const v8::AccessorInfo& info)
> 
> DOMExceptionStackSetter -> domExceptionStackSetter
> 
> > Source/WebCore/bindings/v8/V8ThrowException.cpp:58
> > +        const char* message = 0;
> > +        return V8ThrowException::throwTypeError(message, isolate);
> 
> I would have combined these lines.
> 
> > Source/WebCore/bindings/v8/V8ThrowException.h:32
> > +// The types of javascript errors that can be thrown.
> 
> This comment seems useless and probably should be removed.

Fixed. Thank you very much for a bunch of reviews!