Bug 67991 - Web Inspector: [v8] building call frame info for location-less internal script function crashes.
Summary: Web Inspector: [v8] building call frame info for location-less internal scrip...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-13 01:57 PDT by Pavel Feldman
Modified: 2011-09-14 02:28 PDT (History)
3 users (show)

See Also:


Attachments
Patch (7.22 KB, patch)
2011-09-13 02:35 PDT, Pavel Feldman
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2011-09-13 01:57:23 PDT
Patch to follow.
Comment 1 Pavel Feldman 2011-09-13 02:35:59 PDT
Created attachment 107157 [details]
Patch
Comment 2 Ilya Tikhonovsky 2011-09-13 02:46:14 PDT
Comment on attachment 107157 [details]
Patch

lgtm
Comment 3 Yury Semikhatsky 2011-09-13 03:35:13 PDT
Comment on attachment 107157 [details]
Patch

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

> Source/WebCore/bindings/v8/ScriptDebugServer.cpp:254
> +    v8::TryCatch tryCatch;

What's the purpose of the trycatch here? The next call is not supposed to throw
Comment 4 Pavel Feldman 2011-09-13 04:16:18 PDT
> What's the purpose of the trycatch here? The next call is not supposed to throw

I'm fixing the reproducible crash originating from the next call. Although it does not crash anymore due to the changes in the DebuggerScript.js, I'd like to put the try / catch here in order to cover potential changes to the javascript API that DebuggerScript is using.
Comment 5 Yury Semikhatsky 2011-09-13 11:39:48 PDT
(In reply to comment #4)
> > What's the purpose of the trycatch here? The next call is not supposed to throw
> 
> I'm fixing the reproducible crash originating from the next call. Although it does not crash anymore due to the changes in the DebuggerScript.js, I'd like to put the try / catch here in order to cover potential changes to the javascript API that DebuggerScript is using.

Putting try/catch there will only make finding such problems, I suggest you revert this part.
Comment 6 Pavel Feldman 2011-09-14 02:28:41 PDT
Committed r95083: <http://trac.webkit.org/changeset/95083>