REGRESSION: Web Inspector hangs for many seconds when trying to reload page

[Joseph Pecoraro]

* SUMMARY
Inspector hangs for many seconds when trying to reload page.

* STEPS TO REPRODUCE
1. Inspect cnn.com
2. Show Timeline tab
3. Reload
  => hangs for quite a while, may never show data

* NOTES
- Samples show lots of time spent under Inspector::ScriptDebugServer::sourceParsed (ContentSearchUtilities)
- Seems like this may have been exposed by these recent changes:
  - <https://webkit.org/b/148347> REGRESSION(r188792): broke lots of tests, ggaren is going to investigate and reland (Requested by thorton on #webkit).
  - <https://webkit.org/b/148280> Unify code paths for manually deleting all code

* RELATED:
- We could eliminate the inspector's regex searching by having the JS Parser parse SourceURL / SourceURLMapping comments. This information would also be beneficial for expiation messages in the engine itself (e.g. exceptions in eval code).

[Radar WebKit Bug Importer]

<rdar://problem/23081124>

[Joseph Pecoraro]

It sounds like each time a Debugger is attached to a JSGlobalObject it will send sourceParsed for each source provider. Seems like we could be getting duplicate messages then. Since on a Web Page the same Debugger is shared across all Frames / Isolated Worlds.

[Joseph Pecoraro]

(In reply to comment #2)
> It sounds like each time a Debugger is attached to a JSGlobalObject it will
> send sourceParsed for each source provider. Seems like we could be getting
> duplicate messages then. Since on a Web Page the same Debugger is shared
> across all Frames / Isolated Worlds.

Yes, in the case of Safari on OS X as many as 6 or more isolated worlds get debuggers attached when loading a page (Inspector, Reader, Autofill, Extensions). In the case of the debugger being attached to each of these the inspector was re-pushing all scripts seen so far each time. In general I think there are improvements to be made here, but reducing this to just once (the first time a Debugger is attached) brings this back to what looks like original performance.

[Joseph Pecoraro]

Created attachment 262969 [details]
[PATCH] Proposed Fix

[BJ Burg]

Comment on attachment 262969 [details]
[PATCH] Proposed Fix

I suppose we can't test this because of the reloadPage issues in inspector tests?

[Joseph Pecoraro]

(In reply to comment #5)
> Comment on attachment 262969 [details]
> [PATCH] Proposed Fix
> 
> I suppose we can't test this because of the reloadPage issues in inspector
> tests?

I treated this as a performance issue, so I didn't write a test for that.

But now that you mention it, I would be weary of testing for the reload page issues. It would be worth writing tests to ensure the frontend gets a list of all scripts when the inspector _opens_.

[Mark Lam]

Comment on attachment 262969 [details]
[PATCH] Proposed Fix

r=me

[WebKit Commit Bot]

Comment on attachment 262969 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 262969

Committed r191081: <http://trac.webkit.org/changeset/191081>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.