Bug 56602 - [Chromium] Inspector does not always shows transferred size correctly (e.g. gzipped/chunked content)
: [Chromium] Inspector does not always shows transferred size correctly (e.g. g...
Status: RESOLVED FIXED
: WebKit
Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
: 56691
:
  Show dependency treegraph
 
Reported: 2011-03-17 15:38 PST by
Modified: 2012-01-10 05:06 PST (History)


Attachments
Patch (6.10 KB, patch)
2011-03-24 13:01 PST, Vsevolod Vlasov
pfeldman: review-
Review Patch | Details | Formatted Diff | Diff
Patch that does not break chromium (6.57 KB, patch)
2011-03-24 14:09 PST, Vsevolod Vlasov
pfeldman: review+
commit-queue: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Added changelog (7.74 KB, patch)
2011-03-25 13:52 PST, Vsevolod Vlasov
no flags Review Patch | Details | Formatted Diff | Diff
Patch with didReceiveData2 (4.90 KB, patch)
2011-03-29 08:09 PST, Vsevolod Vlasov
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-03-17 15:38:28 PST
After resolving https://bugs.webkit.org/show_bug.cgi?id=19793 inspector supports showing real transfer size (when it is different from content size).
This should be implemented in chromium port.
------- Comment #1 From 2011-03-24 13:01:28 PST -------
Created an attachment (id=86817) [details]
Patch
------- Comment #2 From 2011-03-24 13:04:47 PST -------
(From update of attachment 86817 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=86817&action=review

> Source/WebKit/chromium/public/WebDevToolsAgent.h:97
> +    virtual void didReceiveData(unsigned long resourceId, int length, int lengthReceived) = 0;

This will break upon the roll. You should make 3 patches:
1) add new method, preserve old method
- wait until deps are rolled
2) migrate chromium to the new method
3) nuke old method upstream
------- Comment #3 From 2011-03-24 14:09:06 PST -------
Created an attachment (id=86833) [details]
Patch that does not break chromium

I have started try bots to be sure this patch does not break chromium, I will write once they have finished.
------- Comment #4 From 2011-03-24 16:08:57 PST -------
One test failed on win bot, but these ExtensionApiTests fail frequently.

http://build.chromium.org/p/tryserver.chromium/builders/win/builds/22928
------- Comment #5 From 2011-03-25 11:37:18 PST -------
(From update of attachment 86833 [details])
Rejecting attachment 86833 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'la..." exit_code: 2

Last 500 characters of output:
9b69f455c6e1d456fe987a8ec14aeb1caed763c7
r81968 = 99f2897a0668627b2e1a5ad2ae3fa0e53ba784c5
r81969 = fede50ddeb05dab38f40fb34c6a3e57680a34e15
r81970 = 66e1a3a6619dbe9d9f04793001909735c77d07ea
r81971 = 80a4fce588aca60b144f21524d2518fab7e5d001
r81972 = 5869263c747ee704ae7a9976eea0082a48153085
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://queues.webkit.org/results/8252177
------- Comment #6 From 2011-03-25 13:52:38 PST -------
Created an attachment (id=86980) [details]
Added changelog
------- Comment #7 From 2011-03-28 22:54:19 PST -------
(From update of attachment 86980 [details])
Clearing flags on attachment: 86980

Committed r82195: <http://trac.webkit.org/changeset/82195>
------- Comment #8 From 2011-03-28 22:54:25 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #9 From 2011-03-29 03:34:57 PST -------
It seems this change broke Chromium Clang builds. I'm rolling out this change...
------- Comment #10 From 2011-03-29 03:35:28 PST -------
Error message:

In file included from content/renderer/render_view.cc:195:
./webkit/plugins/npapi/webplugin_impl.h:85:16: error: 'webkit::npapi::WebPluginImpl::didReceiveData' hides overloaded virtual function [-Werror,-Woverloaded-virtual]
  virtual void didReceiveData(const char* data, int data_length);
               ^
In file included from content/renderer/render_view.cc:181:
In file included from ./webkit/glue/image_resource_fetcher.h:10:
In file included from ./webkit/glue/resource_fetcher.h:22:
./third_party/WebKit/Source/WebKit/chromium/public/WebURLLoaderClient.h:61:18: note: hidden overloaded virtual function 'WebKit::WebURLLoaderClient::didReceiveData' declared here
    virtual void didReceiveData(WebURLLoader*, const char* data, int dataLength, int lengthReceived) { }
                 ^
1 error generated.
make: *** [out/Debug/obj.target/content_renderer/content/renderer/render_view.o] Error 1
------- Comment #11 From 2011-03-29 03:38:23 PST -------
Reverted r82195 for reason:

Broke Chromium Clang builds

Committed r82223: <http://trac.webkit.org/changeset/82223>
------- Comment #12 From 2011-03-29 08:09:39 PST -------
Created an attachment (id=87333) [details]
Patch with didReceiveData2
------- Comment #13 From 2011-03-29 08:33:51 PST -------
(From update of attachment 87333 [details])
Clearing flags on attachment: 87333

Committed r82257: <http://trac.webkit.org/changeset/82257>
------- Comment #14 From 2011-03-29 08:34:01 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #15 From 2011-03-29 10:33:07 PST -------
The commit-queue encountered the following flaky tests while processing attachment 87333 [details]:

fast/workers/storage/use-same-database-in-page-and-workers.html bug 50995 (author: dumi@chromium.org)
The commit-queue is continuing to process your patch.
------- Comment #16 From 2011-04-05 11:48:01 PST -------
(From update of attachment 87333 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=87333&action=review

> Source/WebKit/chromium/public/WebURLLoaderClient.h:61
> +    // FIXME(vsevik): rename once original didReceiveData() is removed.

nit: webkit style is to not put names next to FIXME.

nit: you should document lengthReceived.  it is not obvious how dataLength and lengthReceived differ.
if you read the comment "Called when a chunk of response data is received", then you might think that
lengthReceived indicates the amount received this time, but actually that is dataLength.  also, there
is no indication that lengthReceived somehow relates to the length before the data was decompressed.
------- Comment #17 From 2012-01-09 06:23:17 PST -------
I notice that there a bunch of skipped tests in Chromium's test_expectations.txt, which refer to this bug:

BUGWK56602 SKIP : http/tests/inspector/network/network-timing.html = FAIL
BUGWK56602 SKIP : http/tests/inspector/network/network-size.html = FAIL
BUGWK56602 SKIP : http/tests/inspector/network/network-size-chunked.html = FAIL
BUGWK56602 SKIP : http/tests/inspector/network/network-size-sync.html = FAIL

Now that this bug is marked as fixed, should these tests still be skipped?
------- Comment #18 From 2012-01-10 05:06:31 PST -------
(In reply to comment #17)
> I notice that there a bunch of skipped tests in Chromium's test_expectations.txt, which refer to this bug:
> 
> BUGWK56602 SKIP : http/tests/inspector/network/network-timing.html = FAIL
> BUGWK56602 SKIP : http/tests/inspector/network/network-size.html = FAIL
> BUGWK56602 SKIP : http/tests/inspector/network/network-size-chunked.html = FAIL
> BUGWK56602 SKIP : http/tests/inspector/network/network-size-sync.html = FAIL
> 
> Now that this bug is marked as fixed, should these tests still be skipped?

They should be kept skipped as DRT does not support transferred time/size in inspector.
I added a comment in test expectations and marked them as WONTFIX.