Bug 83122

Summary:

Web Inspector: make padding and client window width part of timeline calculator's state

Product:

WebKit

Reporter:

Andrey Kosyakov <caseq>

Component:

Web Inspector (Deprecated)

Assignee:

Andrey Kosyakov <caseq>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, bweinstein, joepeck, kadam, keishi, loislo, pfeldman, pmuellr, pnormand, rik, timothy, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

83146

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	pfeldman: review+

Description Andrey Kosyakov 2012-04-04 01:09:36 PDT

We used to pass client width to calculator's computeBarGraphWindowPosition(), then adjust for possible padding on the left. This often required passing paddingLeft along with the calculator and lead to possible inconsistent values of width used. Also, this makes it difficult to reuse position calculation logic for a single position, not the entire bar.
This adds display window dimensions to calculator's state, so that it's sufficient to pass calculator around and trivial to implement position calculation for single time value.

Comment 1 Andrey Kosyakov 2012-04-04 01:17:47 PDT

Created attachment 135527 [details]
Patch

Comment 2 Andrey Kosyakov 2012-04-04 01:44:50 PDT

Created attachment 135528 [details]
Patch

Comment 3 Pavel Feldman 2012-04-04 02:09:08 PDT

Comment on attachment 135528 [details]
Patch

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

> Source/WebCore/inspector/front-end/TimelinePanel.js:735
> +    this._minWidth = 5;

Please use constants for these

Comment 4 Andrey Kosyakov 2012-04-04 02:27:40 PDT

Committed r113156: <http://trac.webkit.org/changeset/113156>

Comment 5 Philippe Normand 2012-04-04 03:21:01 PDT

(In reply to comment #4)
> Committed r113156: <http://trac.webkit.org/changeset/113156>

This patch broke 4 inspector tests on GTK...
http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r113156%20(21569)/results.html

Comment 6 Philippe Normand 2012-04-04 03:22:28 PDT

(In reply to comment #5)
> (In reply to comment #4)
> > Committed r113156: <http://trac.webkit.org/changeset/113156>
> 
> This patch broke 4 inspector tests on GTK...
> http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r113156%20(21569)/results.html

https://bugs.webkit.org/show_bug.cgi?id=83136

Comment 7 Ádám Kallai 2012-04-04 04:34:31 PDT

These tests are failing on Qt. After r113156.
http://trac.webkit.org/changeset/113156

* http/tests/inspector/network/network-initiator-from-console.html

Diff:
http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r113156%20(35808)/http/tests/inspector/network/network-initiator-from-console-diff.txt

* http/tests/inspector/network/network-sidebar-width.html

Diff:
http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r113156%20(35808)/http/tests/inspector/network/network-sidebar-width-diff.txt

* http/tests/inspector/resource-har-pages.html

Diff:

http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r113156%20(35808)/http/tests/inspector/resource-har-pages-diff.txt

 
* inspector/debugger/source-frame.html

Diff:
http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r113156%20(35808)/inspector/debugger/source-frame-diff.txt

* inspector/debugger/watch-expressions-panel-switch.html

Diff:

http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r113156%20(35808)/inspector/debugger/watch-expressions-panel-switch-diff.txt

* inspector/network-status-non-http.html

Diff:
http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r113156%20(35808)/inspector/network-status-non-http-diff.txt

I will skip these tests until It is fixed.
Thanks for your investigation.
Please unskip these tests if you have fixed it.

Comment 8 Andrey Kosyakov 2012-04-04 05:42:14 PDT

Committed r113177: <http://trac.webkit.org/changeset/113177>