[tracing] Replace %p with %PRIXPTR in the memory dump names

[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=jochen@chromium.org, skyostil@chromium.org

Committed: https://crrev.com/4205d7b56a9ea78e8fc1c229963019edfe02910f
Cr-Commit-Position: refs/heads/master@{#400004}

[ssid, 2016-06-13 20:59:10]

Description was changed from

==========
[tracing] Replace %p with PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p includes 0x on posix and prints the number without
markers in Windows. To make it consistent, %p is replaced with PRIXPTR
with a standard marker "0x".

BUG=613198
==========

to

==========
[tracing] Replace %p with PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p includes 0x on posix and prints the number without
markers in Windows. To make it consistent, %p is replaced with PRIXPTR
with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ssid, 2016-06-13 21:15:50]

Description was changed from

==========
[tracing] Replace %p with PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p includes 0x on posix and prints the number without
markers in Windows. To make it consistent, %p is replaced with PRIXPTR
with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
[tracing] Replace %p with PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make it consistent, %p is replaced with PRIXPTR
with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ssid, 2016-06-13 21:16:16]

Description was changed from

==========
[tracing] Replace %p with PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make it consistent, %p is replaced with PRIXPTR
with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
[tracing] Replace %p with PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ssid, 2016-06-13 21:43:02]

Patchset #1 (id:1) has been deleted

[ssid, 2016-06-13 21:45:27]

Description was changed from

==========
[tracing] Replace %p with PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ssid, 2016-06-13 22:47:01]

ssid@chromium.org changed reviewers:
+ primiano@chromium.org

[ssid, 2016-06-13 22:47:02]

ptal, thanks

[ssid, 2016-06-13 22:47:19]

Description was changed from

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=brettw@chromium.org
==========

[ssid, 2016-06-13 22:50:04]

Description was changed from

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=brettw@chromium.org
==========

to

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ssid, 2016-06-13 22:55:30]

ssid@chromium.org changed reviewers:
+ jochen@chromium.org, reillyg@chromium.org, shess@chromium.org,
skyostil@chromium.org

[ssid, 2016-06-13 22:55:31]

Need owners stamp:

reillyg@ for extensions/

shess@ for sql/

skyostil for cc/

jochen@ for content/ and gin/

[Scott Hess - ex-Googler, 2016-06-13 23:19:50]

lgtm for sql/

but I think this pattern looks brittle.  Alternatives I can think of seem
over-engineered.  So still lgtm, I guess.

[Reilly Grant (use Gerrit), 2016-06-14 07:39:33]

lgtm

[Primiano Tucci (use gerrit), 2016-06-14 08:33:51]

LGTM. Pity that you had to add all those reinterpreter_cast but I don't see any
nicer option unfortunately.

Out of curiosity, and totally unrelated with your change, did you figure how
does that code in dom_storage_area.cc happen to work without the base:: prefix?
Nothing seems to be doing "using base::StringPrintf" in a header (which would be
bad) or "using namespace base" (which would be bad++). I am really puzzled

https://codereview.chromium.org/2067543003/diff/40001/content/browser/dom_sto...
File content/browser/dom_storage/dom_storage_area.cc (right):

https://codereview.chromium.org/2067543003/diff/40001/content/browser/dom_sto...
content/browser/dom_storage/dom_storage_area.cc:353:
base::StringPrintf("dom_storage/%s/0x%" PRIXPTR, url.c_str(),
uh, how is this working today without base? cannot find anything doing using
StringPrintf? To be clear, your +base:: looks good but I am struggling to figure
out how this did compile before?

[Scott Hess - ex-Googler, 2016-06-14 18:33:42]

On 2016/06/14 08:33:51, Primiano Tucci wrote:
> Pity that you had to add all those reinterpreter_cast but I don't see any
> nicer option unfortunately.

Yeah, having to cast and use a very specific format specifier seems like stuff
that it would be nice to abstract away.  In a different world, it might have
made sense to have the API provide CreateDump() variants for "Unique with this
void*" and "Unique with this void* and this other integer", maybe just putting
them as suffixes.  But given the current state of things, that seems like an
intrusive refactor for little benefit.

[ssid, 2016-06-14 20:28:16]

https://codereview.chromium.org/2067543003/diff/40001/content/browser/dom_sto...
File content/browser/dom_storage/dom_storage_area.cc (right):

https://codereview.chromium.org/2067543003/diff/40001/content/browser/dom_sto...
content/browser/dom_storage/dom_storage_area.cc:353:
base::StringPrintf("dom_storage/%s/0x%" PRIXPTR, url.c_str(),
On 2016/06/14 08:33:51, Primiano Tucci wrote:
> uh, how is this working today without base? cannot find anything doing using
> StringPrintf? To be clear, your +base:: looks good but I am struggling to
figure
> out how this did compile before?

So, we figured that C++ adds the namespaces of the arguments passed to function
call to the search space for the function. But, the arguments passed here are
just "this" pointer and DOMStorageArea class is not part of base namespace. But,
it does inherit from a class from base (base::RefCountedThreadSafe), so base::
is added into the search space.
Now it stopped compiling because the argument this is no longer passed.
C++ is magic!

Note that no file that this file includes has a definition of using
base::StringPrintf. But the above explanation seems to hold with various other
test cases.

[ssid, 2016-06-14 20:35:25]

Description was changed from

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=jochen@chromium.org, skyostil@chromium.org
==========

[ssid, 2016-06-14 20:40:47]

ssid@chromium.org changed reviewers:
+ sievers@chromium.org

[ssid, 2016-06-14 20:40:48]

+sievers since jochen@ is travelling. PTAL Thanks

[Primiano Tucci (use gerrit), 2016-06-15 11:09:24]

On 2016/06/14 18:33:42, Scott Hess wrote:
> Yeah, having to cast and use a very specific format specifier seems like stuff
> that it would be nice to abstract away.  In a different world, it might have
> made sense to have the API provide CreateDump() variants for "Unique with this
> void*" and "Unique with this void* and this other integer", maybe just putting
> them as suffixes.  But given the current state of things, that seems like an
> intrusive refactor for little benefit.

So, we thought to this back in the days. Unfortunately there are cases where the
dump provider wants to build up a complex path (e.g.
gpu/clientX/textures/textureY) and it was making the situation quite complex.
I didn't quite feel back then having a variadic version of the API as that felt
like reinventing sptrintf.  but uhm, in retrospective, given the inconsistencies
found now, probably wouldn't have been a terrible idea.

[no sievers, 2016-06-15 18:23:00]

lgtm

[ssid, 2016-06-15 18:31:42]

On 2016/06/15 18:23:00, sievers wrote:
> lgtm

TBRing jochen and sami since it is a minor change. I am willing to clean up in
case any of them have a better proposal for this issue.

[ssid, 2016-06-15 18:31:48]

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

[commit-bot: I haz the power, 2016-06-15 18:32:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2067543003/40001

[commit-bot: I haz the power, 2016-06-15 20:10:06]

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

[commit-bot: I haz the power, 2016-06-15 20:10:07]

Dry run: This issue passed the CQ dry run.

[ssid, 2016-06-15 20:31:41]

The CQ bit was checked by ssid@chromium.org

[commit-bot: I haz the power, 2016-06-15 20:31:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2067543003/40001

[commit-bot: I haz the power, 2016-06-15 20:36:15]

Description was changed from

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=jochen@chromium.org, skyostil@chromium.org
==========

to

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=jochen@chromium.org, skyostil@chromium.org
==========

[commit-bot: I haz the power, 2016-06-15 20:36:17]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-06-15 20:37:43]

Description was changed from

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=jochen@chromium.org, skyostil@chromium.org
==========

to

==========
[tracing] Replace %p with %PRIXPTR in the memory dump names

The tracing dump names for memory-infra were inconsistent across
platforms since %p prints 0x on posix and prints the number without
markers in Windows. To make the names consistent, %p is replaced with
PRIXPTR with a standard marker "0x".

BUG=613198
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=jochen@chromium.org, skyostil@chromium.org

Committed: https://crrev.com/4205d7b56a9ea78e8fc1c229963019edfe02910f
Cr-Commit-Position: refs/heads/master@{#400004}
==========

[commit-bot: I haz the power, 2016-06-15 20:37:44]

Patchset 2 (id:??) landed as
https://crrev.com/4205d7b56a9ea78e8fc1c229963019edfe02910f
Cr-Commit-Position: refs/heads/master@{#400004}