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 317983003: Add a UseCounter for SVGElement.xmlbase (Closed)

Created:
6 years, 6 months ago by philipj_slow
Modified:
6 years, 5 months ago
Reviewers:
pdr., adamk
CC:
blink-reviews, krit, arv+blink, fs, watchdog-blink-watchlist_google.com, ed+blinkwatch_opera.com, f(malita), Inactive, gyuyoung.kim_webkit.org, Stephen Chennney, kouhei+svg_chromium.org, rwlbuis
Visibility:
Public.

Description

Add a UseCounter for SVGElement.xmlbase BUG=341854

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
M Source/core/frame/UseCounter.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/svg/SVGElement.idl View 1 chunk +1 line, -1 line 1 comment Download

Messages

Total messages: 4 (0 generated)
philipj_slow
6 years, 6 months ago (2014-06-05 23:00:02 UTC) #1
pdr.
LGTM https://codereview.chromium.org/317983003/diff/1/Source/core/svg/SVGElement.idl File Source/core/svg/SVGElement.idl (right): https://codereview.chromium.org/317983003/diff/1/Source/core/svg/SVGElement.idl#newcode30 Source/core/svg/SVGElement.idl:30: attribute DOMString xmllang; Can we do the same ...
6 years, 6 months ago (2014-06-05 23:06:59 UTC) #2
adamk
I'll add details on the bug when I get a chance, but this isn't the ...
6 years, 6 months ago (2014-06-06 00:34:58 UTC) #3
pdr.
6 years, 6 months ago (2014-06-06 03:08:56 UTC) #4
On 2014/06/06 00:34:58, adamk wrote:
> I'll add details on the bug when I get a chance, but this isn't the only place
> where xml:base is used by Blink (both in JS-exposed API and in
> resource-fetching). So while this UseCounter is fine, removing xmlbase
wouldn't
> be sufficient to remove all xml:base support from Blink.

This patch does correctly do what the change description says (measures
SVGElement.xmlbase). Now that I've read the bug, I agree with Adam that this
needs to be expanded. Sorry for the false review :/

Powered by Google App Engine
This is Rietveld 408576698