Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(101)

Issue 18097008: Remove unused startIgnoringLeaks and stopIgnoringLeaks functions (Closed)

Created:
7 years, 5 months ago by kangil_
Modified:
7 years, 2 months ago
CC:
blink-reviews, dglazkov+blink, eae+blinkwatch, adamk+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

These functions were used in earlier DumpRenderTree mac port of WebKit project. Please refer to <https://trac.webkit.org/changeset/19364>; for details. As DumpRenderTree has been removed in chromium project, these functions can be removed. So this patch deletes those. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=154211

Patch Set 1 #

Patch Set 2 : Took Adam's comment into consideration #

Patch Set 3 : Patch should be ready for review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -32 lines) Patch
M Source/core/dom/Node.h View 1 2 1 chunk +0 lines, -4 lines 0 comments Download
M Source/core/dom/Node.cpp View 1 2 3 chunks +2 lines, -28 lines 0 comments Download

Messages

Total messages: 12 (0 generated)
kangil_
review please? :)
7 years, 5 months ago (2013-07-14 05:21:16 UTC) #1
abarth-chromium
What is the benefit of this change? Please explain the "why" behind your change in ...
7 years, 5 months ago (2013-07-14 08:45:59 UTC) #2
kangil_
On 2013/07/14 08:45:59, abarth wrote: > What is the benefit of this change? Please explain ...
7 years, 5 months ago (2013-07-15 01:24:10 UTC) #3
kangil_
Done. :) @abarth: Would you please take a look at this patch? Thx in advance!! ...
7 years, 5 months ago (2013-07-15 02:02:57 UTC) #4
esprehn
Nothing ever calls startIgnoringLeaks or stopIgnoringLeaks why don't we just remove these APIs? Are they ...
7 years, 5 months ago (2013-07-15 05:48:29 UTC) #5
abarth-chromium
I'm not sure this CL is worth landing. These seem like diagnostic functions that are ...
7 years, 5 months ago (2013-07-15 06:19:01 UTC) #6
abarth-chromium
To be clear, I'm not opposed to removing these functions, but we should understand their ...
7 years, 5 months ago (2013-07-15 06:19:48 UTC) #7
kangil_
IMHO, we can delete these functions since seems they were used in earlier DumpRenderTree(which is ...
7 years, 5 months ago (2013-07-15 06:51:12 UTC) #8
abarth-chromium
Makes sense. LGTM
7 years, 5 months ago (2013-07-15 07:47:03 UTC) #9
kangil_
On 2013/07/15 07:47:03, abarth wrote: > Makes sense. LGTM Thx, Adam! :)
7 years, 5 months ago (2013-07-15 07:53:18 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kangil.han@samsung.com/18097008/15001
7 years, 5 months ago (2013-07-15 07:53:41 UTC) #11
commit-bot: I haz the power
7 years, 5 months ago (2013-07-15 09:29:23 UTC) #12
Message was sent while issue was closed.
Change committed as 154211

Powered by Google App Engine
This is Rietveld 408576698