RESOLVED FIXED 93792
[V8] Factor out exception related methods of V8Proxy 
https://bugs.webkit.org/show_bug.cgi?id=93792
Summary [V8] Factor out exception related methods of V8Proxy
Kentaro Hara
Reported 2012-08-12 21:36:05 PDT
To remove V8Proxy, we can factor out exception related methods of V8Proxy to a separate file.
Attachments
Patch (197.38 KB, patch)
2012-08-12 22:24 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from gce-cr-linux-06 (357.21 KB, application/zip)
2012-08-12 23:24 PDT, WebKit Review Bot 		no flags
Details
Patch (197.01 KB, patch)
2012-08-13 01:13 PDT, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kentaro Hara
Comment 1 2012-08-12 22:24:28 PDT
Created attachment 157919 [details] Patch
WebKit Review Bot
Comment 2 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
WebKit Review Bot
Comment 3 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
Kentaro Hara
Comment 4 2012-08-13 01:13:09 PDT
Created attachment 157937 [details] Patch
Adam Barth
Comment 5 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.
Kentaro Hara
Comment 6 2012-08-13 19:04:47 PDT
Committed r125495: <http://trac.webkit.org/changeset/125495>
Kentaro Hara
Comment 7 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!
Note You need to log in before you can comment on or make changes to this bug.