Bug 89567 – Web Inspector: don't report context ids before DidCommitLoad

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

[Yury Semikhatsky]

PageRuntimeAgent::restore() will unconditionally inspected page's frame tree and report context ids for all frames, this may trigger initialization of the contexts if DidCommitLoad event hasn't happened yet. This may lead to some unexpected results like the one described in http://crbug.com/131623

[Yury Semikhatsky]

Created attachment 148554 [details]
Patch

[Pavel Feldman]

Comment on attachment 148554 [details]
Patch

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

> Source/WebCore/inspector/PageRuntimeAgent.cpp:89
> +    if (m_inspectorAgent->didCommitLoadFired()) {

I would instead delegate original didCommitLoad instrumentation here and stored the state (i.e. copied the inspectorAgent's logic).

[Yury Semikhatsky]

Comment on attachment 148554 [details]
Patch

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

>> Source/WebCore/inspector/PageRuntimeAgent.cpp:89
>> +    if (m_inspectorAgent->didCommitLoadFired()) {
> 
> I would instead delegate original didCommitLoad instrumentation here and stored the state (i.e. copied the inspectorAgent's logic).

This would require making this agent always enabled, otherwise we would miss didCommitLoad event. InspectorAgent is an agent that doesn't depend on other agents and is always enabled so it is a good place for storing the flag.

[Pavel Feldman]

Ok, lets them simply move m_didCommitLoadFired into the InspectorPageAgent. It seems more appropriate and is only user there. It would remove the InspectorPageAgent->InspectorAgent dependency as well.

[Yury Semikhatsky]

Committed r120822: <http://trac.webkit.org/changeset/120822>

[Jeffrey Yasskin]

Where's the test to make sure this doesn't regress in the future?

[Yury Semikhatsky]

(In reply to comment #6)
> Where's the test to make sure this doesn't regress in the future?

A test will go downstream since we can only reproduce it with Chromium extensions at the moment.