Don't allow direct const char* trace value

Remove TraceStringWithCopy::operator const char*()

Remove it to avoid misuse like the following:
  TRACE_EVENT1("a", "b", "c",
     condition ? TRACE_STR_COPY(short_live_string) : "literal");
In the above case, the argument value is actually const char*, not
TraceValueWithCopy because it is automatically converted back to
const char* with the const char* operator.

Found and fixed one error in WebKit.

Also changed some TRACE_STR_COPY(cstring.ascii().data()) to
cstring.ascii() because we always copy CStrings.

R=dsinclair,haraken
TBR=... changes of callers about TRACE_STR_NO_COPY
BUG=382789
(perhaps the reason)

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176375

[Xianzhu, 2014-06-17 19:47:13]

https://codereview.chromium.org/342513003/diff/1/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (left):

https://codereview.chromium.org/342513003/diff/1/Source/web/WebViewImpl.cpp#o...
Source/web/WebViewImpl.cpp:1864: TRACE_EVENT1("input",
"WebViewImpl::handleInputEvent", "type", inputEventName ?
TRACE_STR_COPY(inputEventName->ascii().data()) : "unknown");
Because of the original "const char*" operator of TraceStringWithCopy, the
compiler actually pass a "const char*" parameter to TRACE_EVENT1, causing
inputEventName->ascii().data() not copied.

Change to use inputEventName->ascii() directly so that the compiler will pass
CString instead (for which TRACE_EVENT will copy the parameter).

[dsinclair, 2014-06-17 20:47:16]

https://codereview.chromium.org/342513003/diff/1/Source/platform/TraceEvent.h
File Source/platform/TraceEvent.h (right):

https://codereview.chromium.org/342513003/diff/1/Source/platform/TraceEvent.h...
Source/platform/TraceEvent.h:745: template <bool Copy>
Can we make the template parameter an enum? CopyString, NoCopyString?

https://codereview.chromium.org/342513003/diff/1/Source/platform/TraceEvent.h...
Source/platform/TraceEvent.h:811: static inline void setTraceValue(const char
(&arg)[N], unsigned char* type, unsigned long long* value)
This is going to end up stamping out a function for every length of string that
we pass into trace event. That seems like it could get expensive?

[Xianzhu, 2014-06-17 21:27:35]

https://codereview.chromium.org/342513003/diff/1/Source/platform/TraceEvent.h
File Source/platform/TraceEvent.h (right):

https://codereview.chromium.org/342513003/diff/1/Source/platform/TraceEvent.h...
Source/platform/TraceEvent.h:745: template <bool Copy>
On 2014/06/17 20:47:16, dsinclair wrote:
> Can we make the template parameter an enum? CopyString, NoCopyString?

Done with a slightly different method using TraceStringWithCopy and
TraceStringWithoutCopy. In this method, the value of the template parameter
actually doesn't matter, while the typedefed types really matters.

https://codereview.chromium.org/342513003/diff/1/Source/platform/TraceEvent.h...
Source/platform/TraceEvent.h:811: static inline void setTraceValue(const char
(&arg)[N], unsigned char* type, unsigned long long* value)
On 2014/06/17 20:47:16, dsinclair wrote:
> This is going to end up stamping out a function for every length of string
that
> we pass into trace event. That seems like it could get expensive?

There should be no difference before and after the change, because the functions
are inlined. Each setTraceValue call site is expanded to 2 or 3 assignments
despite this change (TRACE_STR_NO_COPY() and TRACE_STR_COPY() are just no-op in
optimized code).

[dsinclair, 2014-06-17 22:14:45]

lgtm

[Xianzhu, 2014-06-17 22:19:49]

On 2014/06/17 22:14:45, dsinclair wrote:
> lgtm

Thanks. Please also review the similar chromium-side change:
https://codereview.chromium.org/338653005/ :)

[Xianzhu, 2014-06-17 23:03:41]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-06-17 23:03:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/342513003/20001

[haraken, 2014-06-17 23:34:14]

LGTM for bindings/

[Xianzhu, 2014-06-18 00:12:25]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2014-06-18 00:28:32]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-06-18 00:29:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/342513003/40001

[commit-bot: I haz the power, 2014-06-18 01:34:42]

Change committed as 176375