Bug 155731 - Web Inspector: New icon for Heap Allocations timeline
Summary: Web Inspector: New icon for Heap Allocations timeline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-03-21 13:31 PDT by Matt Baker
Modified: 2016-03-21 17:51 PDT (History)
10 users (show)

See Also:


Attachments
[Image] Timelines tree icons (52.28 KB, image/png)
2016-03-21 13:31 PDT, Matt Baker
no flags Details
[Patch] Proposed Fix (3.64 KB, patch)
2016-03-21 13:35 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
[Patch] Proposed Fix (4.48 KB, patch)
2016-03-21 16:57 PDT, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2016-03-21 13:31:37 PDT
Created attachment 274618 [details]
[Image] Timelines tree icons

* SUMMARY
New icon for Heap Allocations timeline. Same color scheme as Memory timeline, different graphic.
Comment 1 Radar WebKit Bug Importer 2016-03-21 13:33:41 PDT
<rdar://problem/25275494>
Comment 2 Matt Baker 2016-03-21 13:35:22 PDT
Created attachment 274620 [details]
[Patch] Proposed Fix
Comment 3 Joseph Pecoraro 2016-03-21 13:47:17 PDT
Comment on attachment 274620 [details]
[Patch] Proposed Fix

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

r=me!

> Source/WebInspectorUI/UserInterface/Images/HeapAllocationsInstrument.svg:4
> +    <circle id="Oval" fill="rgb(224, 166, 133)" cx="8" cy="8" r="7.5" stroke="rgb(179, 114, 76)"/>

I think the `id` here can be dropped.

> Source/WebInspectorUI/UserInterface/Views/TimelineIcons.css:40
> +.heap-allocations-icon .icon {
> +    content: url(../Images/HeapAllocationsInstrument.svg);
> +}

Please add a fallback for GTK, see the code just below this (line 50-80) covered by bug 153892. Scripts is probably a reasonable fallback for them in the meantime.

body:not(.mac-platform, .windows-platform) .heap-allocations-icon .icon {
    content: -webkit-image-set(url(../Images/ScriptLarge.png) 1x, url(../Images/ScriptLarge@2x.png) 2x);
}
Comment 4 Timothy Hatcher 2016-03-21 13:51:59 PDT
Comment on attachment 274620 [details]
[Patch] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Images/HeapAllocationsInstrument.svg:3
> +<svg viewBox="0 0 16 16" version="1.1" xmlns="http://www.w3.org/2000/svg">

This should be:
<svg xmlns="http://www.w3.org/2000/svg" id="root" version="1.1" viewBox="0 0 16 16">

Which is the order we use for other SVGs and id="root" incase we ever need to use it as a <symbol>.
Comment 5 Matt Baker 2016-03-21 16:57:14 PDT
Created attachment 274630 [details]
[Patch] Proposed Fix
Comment 6 Joseph Pecoraro 2016-03-21 16:59:41 PDT
Comment on attachment 274630 [details]
[Patch] Proposed Fix

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

r=me

> Source/WebInspectorUI/UserInterface/Views/TimelineIcons.css:81
> +body:not(.mac-platform, .windows-platform) .memory-icon .icon {
> +    content: -webkit-image-set(url(../Images/ScriptLarge.png) 1x, url(../Images/ScriptLarge@2x.png) 2x);
> +}

Heh, thanks. I neglected to add this because EANBLE_RESOURCE_USAGE is not enabled on other ports yet.
Comment 7 Matt Baker 2016-03-21 17:01:37 PDT
(In reply to comment #6)
> Comment on attachment 274630 [details]
> [Patch] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=274630&action=review
> 
> r=me
> 
> > Source/WebInspectorUI/UserInterface/Views/TimelineIcons.css:81
> > +body:not(.mac-platform, .windows-platform) .memory-icon .icon {
> > +    content: -webkit-image-set(url(../Images/ScriptLarge.png) 1x, url(../Images/ScriptLarge@2x.png) 2x);
> > +}
> 
> Heh, thanks. I neglected to add this because EANBLE_RESOURCE_USAGE is not
> enabled on other ports yet.

I was wondering :)
Comment 8 WebKit Commit Bot 2016-03-21 17:51:45 PDT
Comment on attachment 274630 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 274630

Committed r198511: <http://trac.webkit.org/changeset/198511>
Comment 9 WebKit Commit Bot 2016-03-21 17:51:49 PDT
All reviewed patches have been landed.  Closing bug.