Closed Bug 1035978 Opened 10 years ago Closed 10 years ago

Layers/displaylist dumps don't print newline

Categories

(Core :: Graphics: Layers, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
+++ This bug was initially created as a clone of Bug #1030245 +++

Looks like adb logcat will implicitly add a new line if it's missing from the end but this doesn't happen on the desktop or the profiler.
Attachment #8452490 - Flags: review?(bugmail.mozilla)
Attachment #8452490 - Attachment is patch: true
Attachment #8452490 - Attachment mime type: text/x-patch → text/plain
CCing 21 since he hit this bug.
Comment on attachment 8452490 [details] [diff] [review]
patch

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

This feels too clunky to me. If getline() doesn't tack on the \n then we should just do that in the printf_stderr call. It sounded like the problem then is that we get a blank line at the end of the dump, which we should be able to strip out like so:

     while (!ss.eof()) {
       ss.getline(line, sizeof(line));
-      printf_stderr("%s", line);
+      if (!ss.eof() || strlen(line) > 0) {
+        printf_stderr("%s\n", line);
+      }
       if (ss.fail()) {
         // line was too long, skip to next newline
         ss.clear();
         ss.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
       }
     }
Attachment #8452490 - Flags: review?(bugmail.mozilla) → review-
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → bgirard
Attachment #8452490 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8452590 - Flags: review?(bugmail.mozilla)
Comment on attachment 8452590 [details] [diff] [review]
patch

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

r=me with thing below fixed

::: layout/base/nsLayoutUtils.cpp
@@ +3001,5 @@
>      while (!ss.eof()) {
>        ss.getline(line, sizeof(line));
> +      if (!ss.eof() || strlen(line) > 0) {
> +        printf_stderr("%s\n", line);
> +      }

Presumably you want to keep the fprintf_stderr(gfxUtils::sDumpPaintFile...
Attachment #8452590 - Flags: review?(bugmail.mozilla) → review+
Attached patch patch v2Splinter Review
fixed
Attachment #8452590 - Attachment is obsolete: true
Attachment #8452616 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/e7a2beb507f5
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: