Bug 31375

Summary: Web Inspector: breakpoints in named evals are not restored after a reload
Product: WebKit Reporter: Patrick Mueller <pmuellr>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, commit-queue, joepeck, keishi, pfeldman, pmuellr, rik, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
preliminary patch
none
proposed patch 2009/11/12 - a none

Description Patrick Mueller 2009-11-11 14:13:52 PST
If you have "named evals", by virtual of using the //@sourceURL= comment annotation, breakpoints set in these scripts are not restored when the original page is reloaded.  This is a problem with frameworks like Dojo that use evals() for module loading during development.

The breakpoint sidebar will (I believe) show previously set breakpoints after a reload, but they're zombies.  Clicking on them will not take you to the source.  The breakpoints are also not actually set in the source anyway.  So there's no way to get rid of these zombie breakpoints.
Comment 1 Timothy Hatcher 2009-11-11 14:15:36 PST
We could try to reinstall them by matching up the @sourceURL.
Comment 2 Patrick Mueller 2009-11-11 14:28:03 PST
Created attachment 43002 [details]
preliminary patch

The patches are actually simpler than that - the code was pretty much set up to handle this, except the calculated sourceURL wasn't used in ScriptsPanel.addScript(), and in the same method the _sourceIDMap[] map entry is simply set too late for the big if block to take advantage of it.

I'm reusing an existing manual test case to test this, but haven't changed the manual description; I'll do that tomorrow, but thought folks might want to see the current patch.  Looks reasonable.

Also note, I was planning on reorging the breakpoint stuff with this change, but now I'm thinking that I should just do the re-org in a separate patch.  I suspect it will be messy.
Comment 3 Timothy Hatcher 2009-11-11 14:56:05 PST
This patch looks good. Make a ChangeLog and I will r+ it.
Comment 4 Patrick Mueller 2009-11-12 07:16:32 PST
Created attachment 43063 [details]
proposed patch 2009/11/12 - a

per previous comment:

- in ScriptsPanel::addScript(), changed to use the calculated sourceURL rather than the original sourceURL.  The calculated one will have an actual name if the script was an eval that used the //@sourceURL= annotation comment

- in ScriptsPanel::addScript(), moved the cache-filling of _sourceIDmap[] to ABOVE the place it's used, later, in a called function, to paint the breakpoint marker in the source panel; without this fix, everything works, but the breakpoint markers are not repainted for these breakpoints

- added a new manual test case
Comment 5 WebKit Commit Bot 2009-11-12 09:00:31 PST
Comment on attachment 43063 [details]
proposed patch 2009/11/12 - a

Clearing flags on attachment: 43063

Committed r50880: <http://trac.webkit.org/changeset/50880>
Comment 6 WebKit Commit Bot 2009-11-12 09:00:37 PST
All reviewed patches have been landed.  Closing bug.