Layers/displaylist dumps don't print newline

RESOLVED FIXED in mozilla33

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

unspecified
mozilla33
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
Created attachment 8452490 [details] [diff] [review]
patch

+++ 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)
(Assignee)

Updated

4 years ago
Attachment #8452490 - Attachment is patch: true
Attachment #8452490 - Attachment mime type: text/x-patch → text/plain
(Assignee)

Comment 1

4 years ago
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-
(Assignee)

Comment 3

4 years ago
Created attachment 8452590 [details] [diff] [review]
patch
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+
(Assignee)

Comment 5

4 years ago
Created attachment 8452616 [details] [diff] [review]
patch v2

fixed
Attachment #8452590 - Attachment is obsolete: true
Attachment #8452616 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/e7a2beb507f5
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.