New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 638849 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 596760



Sign in to add a comment

Remove WTFLogAlways()

Project Member Reported by tkent@chromium.org, Aug 18 2016

Issue description

WTFLogAlways() should be replaced with LOG() family macros defined in base/logging.h.

https://cs.chromium.org/search/?q=WTFLogAlways&sq=package:chromium&type=cs

Existing callsites:

core/dom/Node.cpp
core/layout/LayoutObject.cpp
core/layout/LayoutGeometryMap.cpp
core/layout/PaintInvalidationState.cpp
core/loader/FrameFetchContext.cpp
platform/graphics/LoggingCanvas.cpp
platform/graphics/paint/PaintController.cpp

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d0702c27c930fc345318da2db6f33b2769bff5c

commit 9d0702c27c930fc345318da2db6f33b2769bff5c
Author: tkent <tkent@chromium.org>
Date: Fri Aug 26 16:12:14 2016

Use Node stream printer in Node::showNode().

This is a preparation to remove WTFLogAlwasy() from Node.cpp.
This CL has no behavior changes.

BUG= 638849 

Review-Url: https://codereview.chromium.org/2282683002
Cr-Commit-Position: refs/heads/master@{#414726}

[modify] https://crrev.com/9d0702c27c930fc345318da2db6f33b2769bff5c/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/9d0702c27c930fc345318da2db6f33b2769bff5c/third_party/WebKit/Source/core/editing/EphemeralRangeTest.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 29 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dea7e1c68164dd101f0c44bd0608379961774edd

commit dea7e1c68164dd101f0c44bd0608379961774edd
Author: tkent <tkent@chromium.org>
Date: Mon Aug 29 10:14:28 2016

Use stream printers in showNodePath() and Node::showTreeForThisAcrossFrame() implementations.

This is a preparation to remove WTFLogAlways() from Node.cpp.
This CL renames the followings:
 - Node::showNodePathForThis() to printNodePathTo()
 - showSubTreeAcrossFrame() to printSubTreeAcrossFrame()
because they don't *show* anything to the console any longer.

This CL has no behavior changes.

BUG= 638849 

Review-Url: https://codereview.chromium.org/2288593003
Cr-Commit-Position: refs/heads/master@{#414991}

[modify] https://crrev.com/dea7e1c68164dd101f0c44bd0608379961774edd/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/dea7e1c68164dd101f0c44bd0608379961774edd/third_party/WebKit/Source/core/dom/Node.h

Comment 3 by pdr@chromium.org, Aug 29 2016

I recently switched all of platform's geometry printers to use toString() in  https://crbug.com/632096 . Do you think a similar call would fit on Node and/or LayoutObject (for a single object, not the whole tree)?

Comment 4 by tkent@chromium.org, Aug 30 2016

#3, Yes.  After #2 CL, I realized introducing Node::toString() was helpful to implement tree printers.  However, as for Node, we need to keep operator<<() too because DCHECK_xx(nodeRef1, nodeRef2) requires it.

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5186386bfed2c08c2253c65652f385859ed3af57

commit 5186386bfed2c08c2253c65652f385859ed3af57
Author: tkent <tkent@chromium.org>
Date: Tue Aug 30 05:26:17 2016

Refactor Node::showTree*() functions.

Update them so that they return strings instead of calling Node::showNode(),
which uses WTFLogAlways().

- Rename showTreeForThis() to toTreeStringForThis()
- Rename showTreeForThisInFlatTree() to toFlatTreeStringForThis()
- Rename showTreeAndMark() to toMarkedTreeString().
- Rename showTreeAndMarkInFlatTree() to toMarkedFlatTreeString()
- Rename traverseTreeAndMark() to appendMarkedTree()
- Rename traverseTreeAndMarkInFlatTree() to appendMarkedFlatTree()
- Add Node::toString() to help these functions.

We'd like to return a string rather than taking a std::ostream argument because
they are used with LOG macros. For example,
  VLOG(1) << node.toMarkedTreeString().utf8().data() << "...";

If we took a std::ostream argument, the code would be:
  std::stringstream buffer;
  node.printMarkedTreeTo(buffer);
  VLOG(1) << buffer.str() << "...";

The former is simpler.

This CL doesn't affect production.

BUG= 638849 

Review-Url: https://codereview.chromium.org/2293703002
Cr-Commit-Position: refs/heads/master@{#415154}

[modify] https://crrev.com/5186386bfed2c08c2253c65652f385859ed3af57/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/5186386bfed2c08c2253c65652f385859ed3af57/third_party/WebKit/Source/core/dom/Node.h
[modify] https://crrev.com/5186386bfed2c08c2253c65652f385859ed3af57/third_party/WebKit/Source/core/dom/Range.cpp
[modify] https://crrev.com/5186386bfed2c08c2253c65652f385859ed3af57/third_party/WebKit/Source/core/editing/Position.cpp
[modify] https://crrev.com/5186386bfed2c08c2253c65652f385859ed3af57/third_party/WebKit/Source/core/editing/VisibleSelection.cpp
[modify] https://crrev.com/5186386bfed2c08c2253c65652f385859ed3af57/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/5186386bfed2c08c2253c65652f385859ed3af57/third_party/WebKit/Source/core/layout/svg/SVGResources.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f0db7b935974863cb125198b6f1e93dfb9b8285

commit 1f0db7b935974863cb125198b6f1e93dfb9b8285
Author: tkent <tkent@chromium.org>
Date: Tue Aug 30 10:01:14 2016

Remove WTFLogAlways() usages from core/{dom,editing}/.

Use LOG(INFO) instead.

For single-line strings like showNode():
  [n:n:n/n:n:INFO:Node.cpp(n)] fooBar

For multiple-line strings like showTree():
  [n:n:n/n:n:INFO:Node.cpp(n)]
  BODY
          #text "\n"
          ...

* This CL removes Node::showNode().  It was a simple wrapper for operator<< for
  Node.

* As for HTMLFormattingElementList and SnapCoordinator, just call LOG(INFO)
  multiple times because they don't show tree structures.

BUG= 638849 

Review-Url: https://codereview.chromium.org/2292843002
Cr-Commit-Position: refs/heads/master@{#415262}

[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/dom/Node.h
[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/editing/Position.cpp
[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/editing/Position.h
[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/editing/VisibleSelection.cpp
[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/html/parser/HTMLElementStack.cpp
[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/html/parser/HTMLFormattingElementList.cpp
[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/1f0db7b935974863cb125198b6f1e93dfb9b8285/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp

Comment 7 by tkent@chromium.org, Aug 30 2016

Components: -Blink>DOM
Done for core/dom/Node.cpp.

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd2ad916a1f9d140b54492987e874de5a1944a0b

commit cd2ad916a1f9d140b54492987e874de5a1944a0b
Author: tkent <tkent@chromium.org>
Date: Thu Sep 01 03:48:30 2016

Remove WTFLogAlways usage in Range.cpp.

Use LOG(INFO) instead.
This CL doesn't affect production.

BUG= 638849 

Review-Url: https://codereview.chromium.org/2295363003
Cr-Commit-Position: refs/heads/master@{#415875}

[modify] https://crrev.com/cd2ad916a1f9d140b54492987e874de5a1944a0b/third_party/WebKit/Source/core/dom/Range.cpp

Comment 9 by w...@chromium.org, Jan 13 2017

tkent: FYI, LOG(INFO) is recommended against - see https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/3NDNd1KzXeY

We should re-convert these new LOG(INFO) instances to VLOG(1) (or higher).

Comment 10 by w...@chromium.org, Jan 13 2017

Cc: w...@chromium.org
Project Member

Comment 11 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b808fcf62211dbab739d4987f0d249524b79780

commit 9b808fcf62211dbab739d4987f0d249524b79780
Author: mrunal.kapade <mrunal.kapade@intel.com>
Date: Thu May 25 09:23:39 2017

Remove WTFLogAlways() usages from HTMLParserScriptRunner.cpp

BUG= 638849 

Review-Url: https://codereview.chromium.org/2906603002
Cr-Commit-Position: refs/heads/master@{#474615}

[modify] https://crrev.com/9b808fcf62211dbab739d4987f0d249524b79780/third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/847692b58cd18ccbfde6efa7167dd3f53716754a

commit 847692b58cd18ccbfde6efa7167dd3f53716754a
Author: Mrunal Kapade <mrunal.kapade@intel.com>
Date: Thu Sep 21 13:56:22 2017

Remove WTFLogAlways() usages from platform/graphics/

Following up from, https://codereview.chromium.org/2905023002.
Use LOG(INFO) instead.

BUG= 638849 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I44fd907ae095489c43b759c5870c83791cd9f341
Reviewed-on: https://chromium-review.googlesource.com/675599
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503435}
[modify] https://crrev.com/847692b58cd18ccbfde6efa7167dd3f53716754a/third_party/WebKit/Source/platform/graphics/LoggingCanvas.cpp
[modify] https://crrev.com/847692b58cd18ccbfde6efa7167dd3f53716754a/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
[modify] https://crrev.com/847692b58cd18ccbfde6efa7167dd3f53716754a/third_party/WebKit/Source/platform/graphics/paint/PaintControllerDebugData.cpp

Comment 13 by tkent@chromium.org, Sep 21 2017

Components: -Blink>Paint -Blink>Loader
Remaining:

core/layout/LayoutObject.cpp
core/layout/LayoutGeometryMap.cpp

See https://codereview.chromium.org/2905733003/

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/950f1d65e0cea3a1712df59ae2ab240c60936a0f

commit 950f1d65e0cea3a1712df59ae2ab240c60936a0f
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Fri Sep 22 17:50:39 2017

Use Utf8().data() for some complex string outputs in LOG

This can keep the original format in the string that was escaped and not
human readable without Utf8().data().

Bug:  638849 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ib0134eade455ec415619861fa9f13b94bb63b890
Reviewed-on: https://chromium-review.googlesource.com/678815
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503790}
[modify] https://crrev.com/950f1d65e0cea3a1712df59ae2ab240c60936a0f/third_party/WebKit/Source/platform/graphics/LoggingCanvas.cpp
[modify] https://crrev.com/950f1d65e0cea3a1712df59ae2ab240c60936a0f/third_party/WebKit/Source/platform/graphics/paint/PaintControllerDebugData.cpp

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/935a591a6ec4e6f64835fd47b8912da71a2b858f

commit 935a591a6ec4e6f64835fd47b8912da71a2b858f
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Sat Oct 21 08:12:12 2017

Remove WTFLogAlways in blink

As a step not to use utilities in Assertions.h, this CL removes both WTFLogAlways() and its all
of uses in the blink. Additionally vprintf_stderr_with_trailing_newline() is also removed because
it has been only using by WTFLogAlways().

BUG= 638849 

Change-Id: Iab934423393de48a09cee2aee6e3703f843c3931
Reviewed-on: https://chromium-review.googlesource.com/729382
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510673}
[modify] https://crrev.com/935a591a6ec4e6f64835fd47b8912da71a2b858f/third_party/WebKit/Source/core/layout/LayoutGeometryMap.cpp
[modify] https://crrev.com/935a591a6ec4e6f64835fd47b8912da71a2b858f/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/935a591a6ec4e6f64835fd47b8912da71a2b858f/third_party/WebKit/Source/platform/wtf/Assertions.cpp
[modify] https://crrev.com/935a591a6ec4e6f64835fd47b8912da71a2b858f/third_party/WebKit/Source/platform/wtf/Assertions.h

Status: Fixed (was: Available)
@tkent, I finally removed WTFLogAlways() in  issue 729382 . So I change the status of this bug with "Fixed". Please correct me if I'm wrong.
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c635753014c72e18e5dcfe0b22625d9cc2aa892

commit 8c635753014c72e18e5dcfe0b22625d9cc2aa892
Author: Kent Tamura <tkent@chromium.org>
Date: Thu Oct 26 01:56:21 2017

Revert "Remove WTFLogAlways in blink"

This reverts commit 935a591a6ec4e6f64835fd47b8912da71a2b858f.

Reason for revert: Output of showLayoutTree(), etc. is ugly.

Original change's description:
> Remove WTFLogAlways in blink
> 
> As a step not to use utilities in Assertions.h, this CL removes both WTFLogAlways() and its all
> of uses in the blink. Additionally vprintf_stderr_with_trailing_newline() is also removed because
> it has been only using by WTFLogAlways().
> 
> BUG= 638849 
> 
> Change-Id: Iab934423393de48a09cee2aee6e3703f843c3931
> Reviewed-on: https://chromium-review.googlesource.com/729382
> Commit-Queue: Gyuyoung Kim <gyuyoung.kim@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510673}

TBR=wangxianzhu@chromium.org,gyuyoung.kim@chromium.org,haraken@chromium.org,tkent@chromium.org,gyuyoung.kim@lge.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  638849 
Change-Id: I0bd4fb7b329dc123d529848b93cad62068d06880
Reviewed-on: https://chromium-review.googlesource.com/737669
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511691}
[modify] https://crrev.com/8c635753014c72e18e5dcfe0b22625d9cc2aa892/third_party/WebKit/Source/core/layout/LayoutGeometryMap.cpp
[modify] https://crrev.com/8c635753014c72e18e5dcfe0b22625d9cc2aa892/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/8c635753014c72e18e5dcfe0b22625d9cc2aa892/third_party/WebKit/Source/platform/wtf/Assertions.cpp
[modify] https://crrev.com/8c635753014c72e18e5dcfe0b22625d9cc2aa892/third_party/WebKit/Source/platform/wtf/Assertions.h

Status: Available (was: Fixed)
Reopen as per #17.
Owner: mrunal.k...@intel.com
Status: Started (was: Available)
Planning to remove last bits of WTFLogAlways with this pending patch, 
https://chromium-review.googlesource.com/c/chromium/src/+/678056
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ce46b96eed1644a16d2660ad4b6633f5cd2e201

commit 1ce46b96eed1644a16d2660ad4b6633f5cd2e201
Author: Mrunal Kapade <mrunal.kapade@intel.com>
Date: Tue Nov 28 00:37:59 2017

Remove WTFLogAlways() usages from core/layout/

Following up from https://codereview.chromium.org/2905733003/
Replace with DLOG(INFO) instead.

BUG= 638849 

Change-Id: I773fba7a2b0c09d5e55e645096eb3c3cad698324
Reviewed-on: https://chromium-review.googlesource.com/678056
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Mrunal Kapade <mrunal.kapade@intel.com>
Cr-Commit-Position: refs/heads/master@{#519480}
[modify] https://crrev.com/1ce46b96eed1644a16d2660ad4b6633f5cd2e201/third_party/WebKit/Source/core/layout/LayoutGeometryMap.cpp
[modify] https://crrev.com/1ce46b96eed1644a16d2660ad4b6633f5cd2e201/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/1ce46b96eed1644a16d2660ad4b6633f5cd2e201/third_party/WebKit/Source/core/layout/LayoutObject.h

Project Member

Comment 23 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddda3c58002a64e7161fc620c5abf2cdf6164a20

commit ddda3c58002a64e7161fc620c5abf2cdf6164a20
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Tue Nov 28 02:55:36 2017

Reland "Remove WTFLogAlways in blink"

This is a reland of 935a591a6ec4e6f64835fd47b8912da71a2b858f.

Original change's description:

>Remove WTFLogAlways in blink

>As a step not to use utilities in Assertions.h, this CL removes both WTFLogAlways() and its all
>of uses in the blink. Additionally vprintf_stderr_with_trailing_newline() is also removed because
>it has been only using by WTFLogAlways().

Bug:  638849 
Change-Id: I7120ca5b70a115bb1c45db57f077254eef57b140
Reviewed-on: https://chromium-review.googlesource.com/792550
Commit-Queue: Gyuyoung Kim <gyuyoung.kim@lge.com>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519538}
[modify] https://crrev.com/ddda3c58002a64e7161fc620c5abf2cdf6164a20/third_party/WebKit/Source/platform/wtf/Assertions.cpp
[modify] https://crrev.com/ddda3c58002a64e7161fc620c5abf2cdf6164a20/third_party/WebKit/Source/platform/wtf/Assertions.h

Status: Fixed (was: Started)

Sign in to add a comment