Network panel does not show main resource content for iframes deleted from the document. chromium bug: https://code.google.com/p/chromium/issues/detail?id=82899 The following change includes InspectorPageAgent refactoring exposing some of the content decoding methods to InspectorResourceAgent. Note that this refactoring will be needed for fixing bugs related to loading the same url in different context (e.g. loading the same url as an image and as a stylesheet).
Created attachment 97484 [details] patch
Comment on attachment 97484 [details] patch Attachment 97484 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/8874304
Comment on attachment 97484 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=97484&action=review The resource buffer extraction changes look sane in general. > Source/WebCore/inspector/InspectorPageAgent.cpp:156 > + return InspectorPageAgent::sharedBufferContent(buffer, textEncodingName, withBase64Encode, result); optional: You might go without the class qualification here and below. Do we have a determined rule for this in the guidelines? > Source/WebCore/inspector/InspectorPageAgent.cpp:159 > +bool InspectorPageAgent::sharedBufferContent(PassRefPtr<SharedBuffer> buffer, const String& textEncodingName, bool withBase64Encode, String* result) For static members, we add the "// static" comment on the preceding line > Source/WebCore/inspector/InspectorPageAgent.cpp:174 > +void InspectorPageAgent::resourceContent(ErrorString* errorString, Frame* frame, const KURL& url, bool base64Encode, String* result) Please add // static above - I must have missed it.
Comment on attachment 97484 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=97484&action=review > LayoutTests/http/tests/inspector/network/network-iframe-load-and-delete.html:12 > + iframe.addEventListener('load', iframeLoaded); iframe.onload = iframeLoaded; > LayoutTests/http/tests/inspector/network/network-iframe-load-and-delete.html:18 > + iframe.removeEventListener('load', iframeLoaded); No need to remove the listener.
Created attachment 97572 [details] patch with build fixes + comments addressed
(In reply to comment #3) > (From update of attachment 97484 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=97484&action=review > > The resource buffer extraction changes look sane in general. > > > Source/WebCore/inspector/InspectorPageAgent.cpp:156 > > + return InspectorPageAgent::sharedBufferContent(buffer, textEncodingName, withBase64Encode, result); > > optional: You might go without the class qualification here and below. Do we have a determined rule for this in the guidelines? According to what I see in MainResourceLoader I removed class qualification for InspectorPageAgent::resourceContent() call. This one, however, is called from file static method, so class qualification could not be omitted.
Comment on attachment 97572 [details] patch with build fixes + comments addressed Attachment 97572 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/8876564
Created attachment 97586 [details] build fixes
Comment on attachment 97586 [details] build fixes View in context: https://bugs.webkit.org/attachment.cgi?id=97586&action=review > Source/WebCore/inspector/InspectorPageAgent.cpp:160 > +bool InspectorPageAgent::sharedBufferContent(PassRefPtr<SharedBuffer> buffer, const String& textEncodingName, bool withBase64Encode, String* result) Consider renaming to non-getter name (fetchSharedBufferConnect). > Source/WebCore/inspector/NetworkResourcesData.h:72 > + InspectorPageAgent::ResourceType type() const { return m_type; } I'd try not to increase the amount of network->page dependencies. Can we keep them separate?
Comment on attachment 97586 [details] build fixes Clearing flags on attachment: 97586 Committed r89163: <http://trac.webkit.org/changeset/89163>
All reviewed patches have been landed. Closing bug.