Closed Bug 1030245 Opened 5 years ago Closed 5 years ago

Layers dump is truncated

Categories

(Core :: Graphics: Layers, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
Attached patch Workaround (obsolete) — Splinter Review
Attachment #8446022 - Flags: feedback?(matt.woodrow)
Attachment #8446022 - Flags: feedback?(matt.woodrow) → feedback?(bgirard)
Attachment #8446022 - Flags: feedback?(bgirard) → feedback+
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.
Attachment #8446022 - Attachment is obsolete: true
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.
Assignee: nobody → bugmail.mozilla
Attachment #8446174 - Attachment is obsolete: true
Attachment #8449624 - Flags: review?(bgirard)
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.
Attachment #8449624 - Flags: review?(bgirard) → review+
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?
Patch is fine as-is. I already have it applied locally where it works great. Let's land this ASAP.
The display list code has the same problem in nsLayoutUtils.cpp.
We should land this as well.
Attachment #8449766 - Flags: review?(bugmail.mozilla)
Attachment #8449766 - Flags: review?(bugmail.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/79c4e94e19b2
https://hg.mozilla.org/mozilla-central/rev/6b16d520f752
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Blocks: 1035978
You need to log in before you can comment on or make changes to this bug.