1030245 - Layers dump is truncated

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Layers dumps going out to printf_stderr on B2G are getting truncated. I suspect this is because now the entire layers dump is built into a single string and output using a single printf_stderr call, and android truncates that call even though it has newlines and whatnot.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Workaround

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Break layer dump into multiple printf_stderr calls

Updated, because the old one got into an infinite loop when an individual line was > 1024 characters. I'm not yet convinced this one doesn't have infinite loops either, because I'm not that familiar with the C++ stream APIs. Just gonna leave this updated workaround here for now.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Break layer dump into multiple printf_stderr calls

Updated so that it doesn't have any infinite loops. I tested this function in isolation using a variety of input on my local (OS X/clang) machine and am reasonably satisfied.

Comment 4

[Benoit Girard (:BenWa)]

Comment on attachment 8449624 [details] [diff] [review]
Break layer dump into multiple printf_stderr calls

Review of attachment 8449624 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM but I wonder if we should be doing this inside printf_stderr when we're going to be priting to android_log so that all users of the API can benefit.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Problem with that is that I'd have to convert the char* back to a stringstream, or do the equivalent operations on a char*. Or I could add a new version of printf_stderr that takes a stringstream.

Honestly it doesn't make sense to put that as a general thing in printf_stderr because it's specific to using a stringstream to build the string. So if there are other places we are using stringstreams (layout debug code, maybe?) then I could add a printf_stderr that takes a stringstream and does this. Otherwise I think landing the patch as-is is fine. Thoughts?

Comment 6

[Benoit Girard (:BenWa)]

Patch is fine as-is. I already have it applied locally where it works great. Let's land this ASAP.

Comment 7

[Benoit Girard (:BenWa)]

The display list code has the same problem in nsLayoutUtils.cpp.

Comment 8

[Benoit Girard (:BenWa)]

Attachment: Break display-list into mutiple printf calls

We should land this as well.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/79c4e94e19b2
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/6b16d520f752

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/79c4e94e19b2
https://hg.mozilla.org/mozilla-central/rev/6b16d520f752