It is the only resource-related information that is available in InspectorController that is missing in the FrameLoaderClient.
Created attachment 29714 [details] patch
Why is this necessary? Please include that kind of information in the future.
(In reply to comment #2) > Why is this necessary? Please include that kind of information in the future. > This is a continuation of https://bugs.webkit.org/show_bug.cgi?id=25342, copying notes from there: Our async/out-of-process version of WebInspector is currently not based on InspectorController. The reason was that we did not want to interfere with Dmitry's unforking effort, yet wanted to experiment. So we came up with these agents concept that basically mimic InspectorController, but separating 'agent' nature from the 'transport'. Now that InspectorController is unforked, I am planning to bring these concepts into the WebKit land and use what we have in Chromium as a proof of concept / experimental playground. It will take me some time to split InspectorController and I don't want Chromium guys to be blocked by that. Hence I want to expose events that are missing in clients, namely: console severity and xmlhttprequest data. I realize that this is somewhat a temporary measure, but I think it will serve us all well.
It will take me some time to split InspectorController and I don't want Chromium guys to be blocked by that
(In reply to comment #4) > It will take me some time to split InspectorController and I don't want > Chromium guys to be blocked by that Blerg, not what I meant to paste. I meant to ask is if this is to help the unforking of chromium or to some other experimental work?
(In reply to comment #5) > (In reply to comment #4) > > It will take me some time to split InspectorController and I don't want > > Chromium guys to be blocked by that > > Blerg, not what I meant to paste. I meant to ask is if this is to help the > unforking of chromium or to some other experimental work? > Here is a broader context: I am currently working on Elements and Resources tabs to allow their async operation. Yury is working on debugger, Mikhail is working on Profiler. The 'work' that they are doing is about making Chromium's internals ready to provide information required and patch/refactor WebInspector to allow async operation in respective areas. Before I dive into splitting InspectorController, I would like to make sure that they are not blocked with what I am doing. As I explained above, our experimental agents are outside WebCore (we needed those to make this thing running out of process and not to clash with Dmitry's effort). But they are missing some information. In particular, Yury is being blocked with the XmlHttp data absence and I am trying to fix this. That is why I need it, but I am sure that having it exposed to loader client is anyway a good thing. Now about our 'experimental' work: it is entirely open, in Chromium repository, located in webkit/glue/devtools folder. It is really not much - a thousand lines of code that mimic InspectorController and establish an RPC-alike interaction with WebInspector. That is something we are using as a proof of concept when bringing code into WebKit.
So, to clarify, this is not something that is need for unforking chromium?
(In reply to comment #7) > So, to clarify, this is not something that is need for unforking chromium? > Sorry, I thought I made it clear. I do confirm, this is not for unforking. This is rather for not forking. Apart from the Chromium business, do you think this change makes sense?
(In reply to comment #8) > (In reply to comment #7) > > So, to clarify, this is not something that is need for unforking chromium? > > > > Sorry, I thought I made it clear. I do confirm, this is not for unforking. This > is rather for not forking. Apart from the Chromium business, do you think this > change makes sense? > Ping. Note, that I've shared a doc/proposal with you (http://docs.google.com/Doc?id=dcf65mvz_0dp6xt4c8) that shows how I think we should approach the InspectorController refactoring. Once this refactoring is done, this change will become obsolete. But as I mentioned before, it will take some time to settle this plan down. In the meanwhile, this change is helpful and does not seem to produce harm (please correct me if I am wrong).
Comment on attachment 29714 [details] patch + virtual void dispatchResourceRetrievedByXMLHttpRequest(unsigned long, const WebCore::ScriptString&); The name for this function does not match other client functions. Something like this would fit better: dispatchDidLoadResourceByXMLHttpRequest + void resourceRetrievedByXMLHttpRequest(unsigned long identifier, const ScriptString& sourceString); This should be renamed to something that matches too. Maybe: didLoadResourceByXMLHttpRequest
Created attachment 29887 [details] patch Updated after Timothy's review.
(In reply to comment #10) > (From update of attachment 29714 [details] [review]) > + virtual void dispatchResourceRetrievedByXMLHttpRequest(unsigned long, > const WebCore::ScriptString&); > > The name for this function does not match other client functions. > > Something like this would fit better: > > dispatchDidLoadResourceByXMLHttpRequest > > + void resourceRetrievedByXMLHttpRequest(unsigned long identifier, const > ScriptString& sourceString); > > This should be renamed to something that matches too. Maybe: > > didLoadResourceByXMLHttpRequest > Done. Thanks for the review!
(In reply to comment #12) > (In reply to comment #10) > > (From update of attachment 29714 [details] [review] [review]) > > + virtual void dispatchResourceRetrievedByXMLHttpRequest(unsigned long, > > const WebCore::ScriptString&); > > > > The name for this function does not match other client functions. > > > > Something like this would fit better: > > > > dispatchDidLoadResourceByXMLHttpRequest > > > > + void resourceRetrievedByXMLHttpRequest(unsigned long identifier, const > > ScriptString& sourceString); > > > > This should be renamed to something that matches too. Maybe: > > > > didLoadResourceByXMLHttpRequest > > > > Done. Thanks for the review! > Ping. Could you give it another look please? Thanks Pavel
Comment on attachment 29887 [details] patch Please add the bug URL to the ChangeLog next time. Add before landing.
Created attachment 29937 [details] patch Added bug url to the ChangeLog.
Created attachment 29938 [details] patch Removed space that squeezed into the method declaration.
Landed as http://trac.webkit.org/changeset/43113.