[Devtools] Network waterfall and grid rows feed bg color from same place

[Devtools] Network waterfall and grid rows feed bg color from same place

Makes network waterfall canvas now feed it's stripe colors from the node
itself rather than deriving it's colors from hard codded property
conditions.

We also removed css background color resolvers.

R=dgozman
BUG=712876
Review-Url: https://codereview.chromium.org/2827843002
Cr-Commit-Position: refs/heads/master@{#465868}
Committed: https://chromium.googlesource.com/chromium/src/+/3d95cd94f4cd59f9ab6603f0cde95d32ecdbed77

[allada, 2017-04-18 22:47:14]

Patchset #1 (id:1) has been deleted

[allada, 2017-04-18 22:52:20]

PTL

[allada, 2017-04-18 22:52:25]

Patchset #1 (id:20001) has been deleted

[pfeldman, 2017-04-18 23:59:46]

pfeldman@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2017-04-18 23:59:47]

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js
(right):

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:57:
const colorUsage = UI.ThemeSupport.ColorUsage;
We don't use const just yet.

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:59:
Default: [255, 255, 255, 1.0],
Extract into enum so that you could refer to them below.

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:66:
var themedColors = /** @type {!Network.NetworkNode._SupportedBackgroundColors}
*/ ({});
The type you define below can't be assigned from {}

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:69:
var patchedColor = Common.Color.parse(UI.themeSupport.patchColor(baseColor,
colorUsage.Background));
It patches the color once you pass it in, so you could just expose a method that
accepts color objects.

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:94:
this._backgroundColorCache = finalColor;
Don't cache it - it does not seem to be expensive.

[allada, 2017-04-19 19:01:23]

Patchset #2 (id:60001) has been deleted

[allada, 2017-04-19 19:01:28]

Patchset #2 (id:80001) has been deleted

[allada, 2017-04-19 19:01:38]

PTaL

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js
(right):

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:57:
const colorUsage = UI.ThemeSupport.ColorUsage;
On 2017/04/18 23:59:46, pfeldman wrote:
> We don't use const just yet.

Done.

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:59:
Default: [255, 255, 255, 1.0],
On 2017/04/18 23:59:46, pfeldman wrote:
> Extract into enum so that you could refer to them below.

Done.

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:66:
var themedColors = /** @type {!Network.NetworkNode._SupportedBackgroundColors}
*/ ({});
On 2017/04/18 23:59:46, pfeldman wrote:
> The type you define below can't be assigned from {}

Done.

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:69:
var patchedColor = Common.Color.parse(UI.themeSupport.patchColor(baseColor,
colorUsage.Background));
On 2017/04/18 23:59:46, pfeldman wrote:
> It patches the color once you pass it in, so you could just expose a method
that
> accepts color objects.

Done.

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:94:
this._backgroundColorCache = finalColor;
On 2017/04/18 23:59:46, pfeldman wrote:
> Don't cache it - it does not seem to be expensive.

Done.

[allada, 2017-04-19 19:01:39]

PTaL

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js
(right):

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:57:
const colorUsage = UI.ThemeSupport.ColorUsage;
On 2017/04/18 23:59:46, pfeldman wrote:
> We don't use const just yet.

Done.

https://codereview.chromium.org/2827843002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:94:
this._backgroundColorCache = finalColor;
On 2017/04/18 23:59:46, pfeldman wrote:
> Don't cache it - it does not seem to be expensive.

Done.

[pfeldman, 2017-04-19 19:52:03]

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/common/Color.js (right):

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/common/Color.js:228: * @param
{boolean=} asCanonical
asCanonical does not say much to me.

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/common/Color.js:265: out_rgb[0] =
hue2rgb(p, q, tr) * multiplier;
Do you want whole numbers here in case of 255?

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js
(right):

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:57:
var colorUsage = UI.ThemeSupport.ColorUsage;
style: You extracted it to use only once.

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:62:
UI.themeSupport.patchHSLA(patchedColor, colorUsage.Background);
Why don't you manipulate colors instead? Isn't this method private?

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:1886: patchHSLA(hsla,
colorUsage) {
I see what you did here. Don't!

[allada, 2017-04-19 23:38:14]

Patchset #3 (id:120001) has been deleted

[allada, 2017-04-19 23:38:46]

PTaL

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/common/Color.js (right):

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/common/Color.js:228: * @param
{boolean=} asCanonical
On 2017/04/19 19:52:03, pfeldman wrote:
> asCanonical does not say much to me.

It's name is modeled after:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/common/Color.js:265: out_rgb[0] =
hue2rgb(p, q, tr) * multiplier;
On 2017/04/19 19:52:02, pfeldman wrote:
> Do you want whole numbers here in case of 255?

Yes.

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js
(right):

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:57:
var colorUsage = UI.ThemeSupport.ColorUsage;
On 2017/04/19 19:52:03, pfeldman wrote:
> style: You extracted it to use only once.

Done.

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/network/NetworkDataGridNode.js:62:
UI.themeSupport.patchHSLA(patchedColor, colorUsage.Background);
On 2017/04/19 19:52:03, pfeldman wrote:
> Why don't you manipulate colors instead? Isn't this method private?

I exposed this method instead. It does exactly what I need. The only thing
that's strange is it manipulates the input parameter. I could change it, but I
figured it does this for speed reasons since timeline is a huge user of this
color stuff and probably wants it fast.

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right):

https://codereview.chromium.org/2827843002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:1886: patchHSLA(hsla,
colorUsage) {
On 2017/04/19 19:52:03, pfeldman wrote:
> I see what you did here. Don't!

Done.

[pfeldman, 2017-04-20 00:44:00]

lgtm

[allada, 2017-04-20 00:49:55]

The CQ bit was checked by allada@chromium.org

[commit-bot: I haz the power, 2017-04-20 00:50:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-20 02:25:09]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1492649395086580,
"parent_rev": "f8b7dc953a79f8565fde8cd22642b000a236d5a5", "commit_rev":
"3d95cd94f4cd59f9ab6603f0cde95d32ecdbed77"}

[commit-bot: I haz the power, 2017-04-20 02:25:16]

Description was changed from

==========
[Devtools] Network waterfall and grid rows feed bg color from same place

Makes network waterfall canvas now feed it's stripe colors from the node
itself rather than deriving it's colors from hard codded property
conditions.

We also removed css background color resolvers.

R=dgozman
BUG=712876
==========

to

==========
[Devtools] Network waterfall and grid rows feed bg color from same place

Makes network waterfall canvas now feed it's stripe colors from the node
itself rather than deriving it's colors from hard codded property
conditions.

We also removed css background color resolvers.

R=dgozman
BUG=712876

Review-Url: https://codereview.chromium.org/2827843002
Cr-Commit-Position: refs/heads/master@{#465868}
Committed:
https://chromium.googlesource.com/chromium/src/+/3d95cd94f4cd59f9ab6603f0cde9...
==========

[commit-bot: I haz the power, 2017-04-20 02:25:17]

Committed patchset #3 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/3d95cd94f4cd59f9ab6603f0cde9...