Bug 40364 – Web Inspector: don't push script sources to frontend if scripts panel is inactive

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Podivilov]

Currently v8 debugger is enabled when frontend is attached (see https://bugs.webkit.org/show_bug.cgi?id=40289).
As a result, script sources are always pushed to frontend even if scripts panel is inactive.

[Yury Semikhatsky]

Pushing only scripts metadata and requesting script source lazily from the front end would reduce traffic between InspectorController and the front end. It would improve remote debugger performance so both engines should benefit from that. Another option would be in case of v8 to push already compiled scripts only when scripts panel is shown or when debugger hits a breakpoint. Any objections to the first approach?

[Yury Semikhatsky]

Created attachment 58378 [details]
Patch

[Pavel Feldman]

Comment on attachment 58378 [details]
Patch

WebCore/inspector/InspectorBackend.h:94
 +      void resolveScriptSource(long callId, const String& sourceID);
getScriptSource? There is not much to resolve there...

WebCore/inspector/InspectorController.cpp:1698
 +      String dataToSend = ScriptDebugServer::shared().isDebuggerAlwaysEnabled() ? "" : data;
Can we make it always lazy regardless the setting?

[Yury Semikhatsky]

Created attachment 58381 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #3)
> (From update of attachment 58378 [details])
> WebCore/inspector/InspectorBackend.h:94
>  +      void resolveScriptSource(long callId, const String& sourceID);
> getScriptSource? There is not much to resolve there...
> 
Done.

> WebCore/inspector/InspectorController.cpp:1698
>  +      String dataToSend = ScriptDebugServer::shared().isDebuggerAlwaysEnabled() ? "" : data;
> Can we make it always lazy regardless the setting?
Done. Now it's lazy no matter what kind of debugger is used. I had some concerns about slowing things down in Safari.

[Yury Semikhatsky]

Created attachment 58384 [details]
Patch

[Pavel Feldman]

Comment on attachment 58384 [details]
Patch

This is scary!

[Yury Semikhatsky]

Comment on attachment 58384 [details]
Patch

Clearing flags on attachment: 58384

Committed r60965: <http://trac.webkit.org/changeset/60965>

[Yury Semikhatsky]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Comment on attachment 58384 [details]
Patch

This looks like a good solution! Nice!

I know this landed already (Sorry I'm so far behind). There
were some typos in the comments, nothing too important. Just
keep an eye out for these in future patches.

> +        script content lazily at the moment it should be displyed. It is critical for

Typo: s/displyed/displayed/;
(this is in each ChangeLog)


> +    // Don't send script content to the front end until it's realy needed.

Typo: s/realy/really/;