Bug 41247 - Web Inspector: ScripsPanel.prototype.editScriptSource uses old addBreakpoint signature.
Summary: Web Inspector: ScripsPanel.prototype.editScriptSource uses old addBreakpoint ...
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: 2010-06-26 04:10 PDT by Pavel Feldman
Modified: 2010-06-26 06:25 PDT (History)
5 users (show)

See Also:

Attachments
[PATCH] Proposed fix. (4.77 KB, patch)
2010-06-26 04:30 PDT, Pavel Feldman 		no flags Details | Formatted Diff | Diff
[PATCH] Style fixed. (4.77 KB, patch)
2010-06-26 04:41 PDT, Pavel Feldman 		yurys: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-06-26 04:10:57 PDT 
I also added code preventing removed breakpoints from generating events.
Comment 1 Pavel Feldman 2010-06-26 04:30:33 PDT 
Created attachment 59829 [details]
[PATCH] Proposed fix.
Comment 2 WebKit Review Bot 2010-06-26 04:32:18 PDT 
Attachment 59829 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/inspector/front-end/Object.js:57:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Pavel Feldman 2010-06-26 04:41:56 PDT 
Created attachment 59830 [details]
[PATCH] Style fixed.
Comment 4 Yury Semikhatsky 2010-06-26 05:51:14 PDT 
Comment on attachment 59830 [details]
[PATCH] Style fixed.

WebCore/inspector/front-end/BreakpointManager.js:151
 +              this._breakpointManager._saveBreakpointOnBackend(this);
These methods should not be called after the breakpoint has been removed. Can you fix the call sites instead?

We need a test for this.
Comment 5 Pavel Feldman 2010-06-26 06:06:43 PDT 
(In reply to comment #4)
> (From update of attachment 59830 [details])
> WebCore/inspector/front-end/BreakpointManager.js:151
>  +              this._breakpointManager._saveBreakpointOnBackend(this);
> These methods should not be called after the breakpoint has been removed. Can you fix the call sites instead?
> 

There are no call sites like this - I was just making sure we don't hit this problem in the future. But I see your point, I'll remove the checks and let code fail now that manager reference is cleared.
Comment 6 Pavel Feldman 2010-06-26 06:25:53 PDT 
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/BreakpointManager.js
	M	WebCore/inspector/front-end/Object.js
	M	WebCore/inspector/front-end/ScriptsPanel.js
Committed r61954