Bug 49130 - Web Inspector: add more event listener breakpoint types, add support for regular breakpoint hit state, beautify hit rendering.
Summary: Web Inspector: add more event listener breakpoint types, add support for regu...
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-11-06 12:55 PDT by Pavel Feldman
Modified: 2010-11-12 08:44 PST (History)
16 users (show)

See Also:


Attachments
[PATCH] Proposed change. (8.24 KB, patch)
2010-11-06 12:58 PDT, Pavel Feldman
yurys: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Layout test fix (2.53 KB, patch)
2010-11-08 11:27 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Improved version of the patch that I'm going to land (9.70 KB, patch)
2010-11-12 08:38 PST, Yury Semikhatsky
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-11-06 12:55:33 PDT
Patch to follow.
Comment 1 Pavel Feldman 2010-11-06 12:58:29 PDT
Created attachment 73174 [details]
[PATCH] Proposed change.
Comment 2 WebKit Commit Bot 2010-11-08 02:01:21 PST
Comment on attachment 73174 [details]
[PATCH] Proposed change.

Rejecting patch 73174 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'apply-attachment', '--force-clean', '--non-interactive', 73174]" exit_code: 2
Last 500 characters of output:
patch 73174 from bug 49130.
Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Yury Semikhatsky', u'--force']" exit_code: 9
Parsed 6 diffs from patch file(s).
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
only literal type is supported now at /Projects/CommitQueue/WebKitTools/Scripts/svn-apply line 248.

Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Yury Semikhatsky', u'--force']" exit_code: 9

Full output: http://queues.webkit.org/results/5515025
Comment 3 Eric Seidel (no email) 2010-11-08 10:50:16 PST
SUCCESS: Build 20455 (r71507) was the first to show failures: set([u'inspector/debugger-pause-on-breakpoint.html', u'inspector/debugger-pause-on-debugger-statement.html'])
Comment 4 Eric Seidel (no email) 2010-11-08 10:50:37 PST
This landed, yes?  It seems to have broken tests.
Comment 5 Andrey Kosyakov 2010-11-08 11:25:17 PST
Rolled back at r71550: http://trac.webkit.org/changeset/71550
This broke inspector/debugger-pause-on-breakpoint.html and inspector/debugger-pause-on-debugger-statement.html
Comment 6 Yury Semikhatsky 2010-11-08 11:27:21 PST
Created attachment 73256 [details]
Layout test fix

Here is a patch fixing debugger test failures appeared after the change above was landed. Chromium expectations still need to be updated.
Comment 7 WebKit Review Bot 2010-11-08 11:41:36 PST
http://trac.webkit.org/changeset/71506 might have broken Leopard Intel Release (Tests) and SnowLeopard Intel Release (Tests)
The following tests are not passing:
inspector/debugger-pause-on-breakpoint.html
inspector/debugger-pause-on-debugger-statement.html
Comment 8 Alexander Pavlov (apavlov) 2010-11-09 08:18:20 PST
Even though this has been rolled back, you might want to rework and re-commit the patch. Just in case, localizedStrings.js is not valid JS as it contains the following line:

localizedStrings["Device" = "Device";
Comment 9 Yury Semikhatsky 2010-11-12 08:38:06 PST
Created attachment 73750 [details]
Improved version of the patch that I'm going to land

Compared to the original change this one:
- places this.sidebarPanes.eventListenerBreakpoints.reset(); under  if (Preferences.nativeInstrumentationEnabled) { guard (ScriptsPanel.js)
- checks that a breakpoint has populateStatusMessageElement method, otherwise will get an exception for JS breakpoints(CallStackSidebarPane.js)

without these modifications we will get exceptions in runtime, these exceptions caused inspector layout tests to fail.
Comment 10 Yury Semikhatsky 2010-11-12 08:42:18 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/inspector/front-end/BreakpointManager.js
	M	WebCore/inspector/front-end/BreakpointsSidebarPane.js
	M	WebCore/inspector/front-end/CallStackSidebarPane.js
	M	WebCore/inspector/front-end/ScriptsPanel.js
	M	WebCore/inspector/front-end/inspector.css
Committed r71917
Comment 11 Joseph Pecoraro 2010-11-12 08:44:45 PST
Comment on attachment 73750 [details]
Improved version of the patch that I'm going to land

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

Cool, glad to see a bunch of other events in the list that are difficult to debug (like unload)!

> WebCore/inspector/front-end/BreakpointsSidebarPane.js:264
> +    // FIXME: uncomment following once inspector stops being drop targer in major ports.

Typo: "targer" => "target"