Bug 95957 - Web Inspector: NMI: switch to hierarchical identifiers of MemoryBlockTypes and use these identifiers in protocol.
Summary: Web Inspector: NMI: switch to hierarchical identifiers of MemoryBlockTypes an...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ilya Tikhonovsky
URL:
Keywords:
Depends on: 95988
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-06 02:42 PDT by Ilya Tikhonovsky
Modified: 2012-09-06 08:43 PDT (History)
14 users (show)

See Also:


Attachments
Patch (17.01 KB, patch)
2012-09-06 02:56 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
Patch (19.38 KB, patch)
2012-09-06 05:52 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
Patch (22.38 KB, patch)
2012-09-06 06:01 PDT, Ilya Tikhonovsky
no flags Details | Formatted Diff | Diff
Patch (21.69 KB, patch)
2012-09-06 06:19 PDT, Ilya Tikhonovsky
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2012-09-06 02:42:38 PDT
EOM
Comment 1 Ilya Tikhonovsky 2012-09-06 02:56:26 PDT
Created attachment 162459 [details]
Patch
Comment 2 Yury Semikhatsky 2012-09-06 05:32:23 PDT
Comment on attachment 162459 [details]
Patch

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

> Source/WebCore/dom/MemoryInstrumentation.cpp:40
> +MemoryObjectType GenericMemoryTypes::OwnersTypePlaceholder = "OwnersTypePlaceholder";

The new name is too verbose and still sounds unclear to me, I'd rather leave it as Other or passed 0 where the Other is used since we should never report types with such type anyways.

> Source/WebCore/dom/MemoryInstrumentation.cpp:42
> +MemoryObjectType WebCoreMemoryTypes::DOMTree = "DOMTree";

DOMTree -> Page ?

> Source/WebCore/dom/MemoryInstrumentation.cpp:50
> +MemoryObjectType WebCoreMemoryTypes::CachedResource = "MemoryCache.RawResource";

CachedResource -> CachedResourceRaw
Comment 3 Ilya Tikhonovsky 2012-09-06 05:52:43 PDT
Created attachment 162486 [details]
Patch
Comment 4 Ilya Tikhonovsky 2012-09-06 06:01:11 PDT
Created attachment 162487 [details]
Patch
Comment 5 Yury Semikhatsky 2012-09-06 06:16:39 PDT
Comment on attachment 162487 [details]
Patch

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

> Source/WebCore/loader/cache/CachedResource.cpp:806
> +    MemoryClassInfo info(memoryObjectInfo, this, GenericMemoryTypes::Undefined);

There may be instances of the CachedResource class(that are not descendants).
Comment 6 Ilya Tikhonovsky 2012-09-06 06:19:31 PDT
Created attachment 162489 [details]
Patch
Comment 7 Ilya Tikhonovsky 2012-09-06 06:23:18 PDT
(In reply to comment #5)
> (From update of attachment 162487 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162487&action=review
> 
> > Source/WebCore/loader/cache/CachedResource.cpp:806
> > +    MemoryClassInfo info(memoryObjectInfo, this, GenericMemoryTypes::Undefined);
> 
> There may be instances of the CachedResource class(that are not descendants).

done
Comment 8 Ilya Tikhonovsky 2012-09-06 07:25:28 PDT
Committed r127738: <http://trac.webkit.org/changeset/127738>
Comment 9 Csaba Osztrogonác 2012-09-06 08:27:35 PDT
(In reply to comment #8)
> Committed r127738: <http://trac.webkit.org/changeset/127738>

It caused a regression - https://bugs.webkit.org/show_bug.cgi?id=95988
Could you check it, please?
Comment 10 Ilya Tikhonovsky 2012-09-06 08:43:05 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Committed r127738: <http://trac.webkit.org/changeset/127738>
> 
> It caused a regression - https://bugs.webkit.org/show_bug.cgi?id=95988
> Could you check it, please?

the fix is landed as r127744