DevTools: migrate ResourcesPanel icons over to UI.Icon

DevTools: migrate ResourcesPanel icons over to UI.Icon

This patch migrates ResourcesPanel over to the UI.Icon. This CL is a bit
longer then usual migration CL's because a few thigns had to happen
simultaneously:
- a new spritesheet is created for the resourceGlyphs
- the treeElement.createIcon() method is no longer needed
- the rest of ResourcesPanel is migrated over to the UI.Icon

BUG=669323
R=dgozman

Review-Url: https://codereview.chromium.org/2660893002
Cr-Commit-Position: refs/heads/master@{#447191}
Committed: https://chromium.googlesource.com/chromium/src/+/4b1dc85cf4ee7711aaf07585ec51249e77cfbfac

[lushnikov, 2017-01-30 12:50:36]

please, take a look

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js
(left):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js:57:
this, Common.UIString('Local Storage'), 'LocalStorage', ['table-tree-item',
'resource-tree-item']);
note: the StorageCategoryTreeElement doesn't accept array and a boolean as a
parameter any more

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js:994:
this._storagePanel, resource.resourceType().category().title, categoryName,
null, true);
this was the only usage of the "noIcon" argument

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js
(right):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/resources/ResourcesPanel.js:943:
super(storagePanel, '', false);
started explicitly passing the "expandable" argument of the constructor

[lushnikov, 2017-01-30 12:50:43]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-30 12:50:52]

Dry run: 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-01-30 15:01:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-30 15:01:21]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-01-30 16:47:53]

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css
(left):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css:116:
-webkit-filter: invert();
Did you regress this or it just works?

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css
(right):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css:29:
.navigator-tree-item {
So, you mix 16x16px icons and 32x24px icons in the same tree. Does it make sense
to have 'largeicon-file-item' and 'mediumicon-file-item', using the same mask
but different positions?

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/Icon.js (right):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Icon.js:227:
'resourceicon-manifest': {x: 0, y: -0, width: 16, height: 16, spritesheet:
'resourceicons', isMask: true},
mediumicon-manifest here and below.

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Icon.js:228:
'resourceicon-service-worker': {x: -20, y: -0, width: 16, height: 16,
spritesheet: 'resourceicons', isMask: true},
The spritesheet itself is missing from CL.

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/treeoutline.js (left):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/treeoutline.js:643: createIcon()
{
Nice!

[lushnikov, 2017-01-31 03:17:32]

thank you for the review! please, take another look

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css
(left):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css:116:
-webkit-filter: invert();
On 2017/01/30 16:47:52, dgozman wrote:
> Did you regress this or it just works?

The .service-workers-tree-item class is never used

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css
(right):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css:29:
.navigator-tree-item {
On 2017/01/30 16:47:52, dgozman wrote:
> So, you mix 16x16px icons and 32x24px icons in the same tree. Does it make
sense
> to have 'largeicon-file-item' and 'mediumicon-file-item', using the same mask
> but different positions?

there are two different trees in Applicaion panel: one in Frames section (it
uses 32x24 icons) and one Storage section (it uses 16x16 icons). The 16x16 icons
come from the resourceGlyphs - they have no relation to toolbarButtonGlyphs. 

I didn't follow your idea with using the same mask but different position -
could you please explain?

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/Icon.js (right):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Icon.js:227:
'resourceicon-manifest': {x: 0, y: -0, width: 16, height: 16, spritesheet:
'resourceicons', isMask: true},
On 2017/01/30 16:47:52, dgozman wrote:
> mediumicon-manifest here and below.

Done.

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Icon.js:228:
'resourceicon-service-worker': {x: -20, y: -0, width: 16, height: 16,
spritesheet: 'resourceicons', isMask: true},
On 2017/01/30 16:47:52, dgozman wrote:
> The spritesheet itself is missing from CL.

The spritesheet is defined in inspectorCommon.css. The resourceGlyphs.png
spritesheet is already existing in tip-of-tree

[lushnikov, 2017-01-31 03:17:39]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 03:18:07]

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

[dgozman, 2017-01-31 03:57:11]

lgtm

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css
(right):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/resources/resourcesSidebar.css:29:
.navigator-tree-item {
On 2017/01/31 03:17:32, lushnikov wrote:
> On 2017/01/30 16:47:52, dgozman wrote:
> > So, you mix 16x16px icons and 32x24px icons in the same tree. Does it make
> sense
> > to have 'largeicon-file-item' and 'mediumicon-file-item', using the same
mask
> > but different positions?
> 
> there are two different trees in Applicaion panel: one in Frames section (it
> uses 32x24 icons) and one Storage section (it uses 16x16 icons). The 16x16
icons
> come from the resourceGlyphs - they have no relation to toolbarButtonGlyphs. 
Thanks for explanation.

> I didn't follow your idea with using the same mask but different position -
> could you please explain?
The idea was that we can use the same glyph for medium-sized and large-sized
icon, just defining different x, y, width and height and leaving different
amount of whitespace around. This way we can replace the margins.

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/ui/Icon.js (right):

https://codereview.chromium.org/2660893002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/ui/Icon.js:228:
'resourceicon-service-worker': {x: -20, y: -0, width: 16, height: 16,
spritesheet: 'resourceicons', isMask: true},
On 2017/01/31 03:17:32, lushnikov wrote:
> On 2017/01/30 16:47:52, dgozman wrote:
> > The spritesheet itself is missing from CL.
> 
> The spritesheet is defined in inspectorCommon.css. The resourceGlyphs.png
> spritesheet is already existing in tip-of-tree

Oh, my bad.

[lushnikov, 2017-01-31 04:00:27]

thanks for the late-night review!

[lushnikov, 2017-01-31 04:00:35]

The CQ bit was unchecked by lushnikov@chromium.org

[lushnikov, 2017-01-31 04:00:35]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2017-01-31 04:00:47]

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-01-31 04:50:12]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485835235914490,
"parent_rev": "b2ba6289f11d85440727609ac8430e7804933066", "commit_rev":
"4b1dc85cf4ee7711aaf07585ec51249e77cfbfac"}

[commit-bot: I haz the power, 2017-01-31 04:50:50]

Description was changed from

==========
DevTools: migrate ResourcesPanel icons over to UI.Icon

This patch migrates ResourcesPanel over to the UI.Icon. This CL is a bit
longer then usual migration CL's because a few thigns had to happen
simultaneously:
- a new spritesheet is created for the resourceGlyphs
- the treeElement.createIcon() method is no longer needed
- the rest of ResourcesPanel is migrated over to the UI.Icon

BUG=669323
R=dgozman
==========

to

==========
DevTools: migrate ResourcesPanel icons over to UI.Icon

This patch migrates ResourcesPanel over to the UI.Icon. This CL is a bit
longer then usual migration CL's because a few thigns had to happen
simultaneously:
- a new spritesheet is created for the resourceGlyphs
- the treeElement.createIcon() method is no longer needed
- the rest of ResourcesPanel is migrated over to the UI.Icon

BUG=669323
R=dgozman

Review-Url: https://codereview.chromium.org/2660893002
Cr-Commit-Position: refs/heads/master@{#447191}
Committed:
https://chromium.googlesource.com/chromium/src/+/4b1dc85cf4ee7711aaf07585ec51...
==========

[commit-bot: I haz the power, 2017-01-31 04:50:51]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4b1dc85cf4ee7711aaf07585ec51...