Bug 83802 - Web Inspector: extracting NetworkRequest from Resource (step 3)
Summary: Web Inspector: extracting NetworkRequest from Resource (step 3)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks: 83893
  Show dependency treegraph
 
Reported: 2012-04-12 12:01 PDT by Pavel Feldman
Modified: 2012-04-13 08:06 PDT (History)
10 users (show)

See Also:


Attachments
Patch (62.27 KB, patch)
2012-04-12 12:07 PDT, Pavel Feldman
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2012-04-12 12:01:13 PDT
This change extracts NetworkRequest from the Resource. It is now clear that these two should have super class that would be responsible for parsing URL and would define the requestContent signature. Corresponding patch will follow.
Comment 1 Pavel Feldman 2012-04-12 12:01:42 PDT
See https://bugs.webkit.org/show_bug.cgi?id=61363 for the meta bug.
Comment 2 Pavel Feldman 2012-04-12 12:07:07 PDT
Created attachment 136950 [details]
Patch
Comment 3 Timothy Hatcher 2012-04-12 12:18:12 PDT
What is the motivation for this change?
Comment 4 Pavel Feldman 2012-04-12 13:03:17 PDT
(In reply to comment #3)
> What is the motivation for this change?

I posted more details in https://bugs.webkit.org/show_bug.cgi?id=61363#c2.
Comment 5 Pavel Podivilov 2012-04-13 04:30:02 PDT
Comment on attachment 136950 [details]
Patch

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

> Source/WebCore/inspector/front-end/RawSourceCode.js:57
> +    if (this._request)

Is it possible that both request and resource exist? If yes, it should be "this._request && !this._resource".

> Source/WebCore/inspector/front-end/RawSourceCode.js:138
> +        if (!this._request || !this._hasNewScripts)

It should probably be !this._resource.
Comment 6 Pavel Feldman 2012-04-13 04:43:04 PDT
(In reply to comment #5)
> (From update of attachment 136950 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=136950&action=review
> 
> > Source/WebCore/inspector/front-end/RawSourceCode.js:57
> > +    if (this._request)
> 
> Is it possible that both request and resource exist? If yes, it should be "this._request && !this._resource".
> 

No, it is not possible.

> > Source/WebCore/inspector/front-end/RawSourceCode.js:138
> > +        if (!this._request || !this._hasNewScripts)
> 
> It should probably be !this._resource.

This is ex-useTemporaryContent, so it is based on request, not resource.
Comment 7 Yury Semikhatsky 2012-04-13 05:00:27 PDT
Comment on attachment 136950 [details]
Patch

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

>> Source/WebCore/inspector/front-end/RawSourceCode.js:57
>> +    if (this._request)
> 
> Is it possible that both request and resource exist? If yes, it should be "this._request && !this._resource".

Let's rename it to _pendingRequest

> Source/WebCore/inspector/front-end/ResourceScriptMapping.js:99
> +                if (request && (request.finished || request.type !== WebInspector.resourceTypes.Document))

request.finished check seems redundant for inflights.
Comment 8 Pavel Feldman 2012-04-13 05:15:30 PDT
Committed r114116: <http://trac.webkit.org/changeset/114116>