update_use_counter_feature_enum.py broken |
||||||
Issue description
I recently tried to use update_use_counter_feature_enum.py and I got the following errors:
Traceback (most recent call last):
File "./update_use_counter_feature_enum.py", line 48, in <module>
end_marker=END_MARKER)
File "/usr/local/google/home/guidou/ssd/projects/chromium/src/tools/metrics/histograms/update_histogram_enum.py", line 234, in UpdateHistogramEnum
source_enum_path)
File "/usr/local/google/home/guidou/ssd/projects/chromium/src/tools/metrics/histograms/update_histogram_enum.py", line 205, in UpdateHistogramFromDict
source_enum_path)
File "/usr/local/google/home/guidou/ssd/projects/chromium/src/tools/metrics/histograms/update_histogram_enum.py", line 167, in _GetOldAndUpdatedXml
source_enum_path, histograms_doc)
File "/usr/local/google/home/guidou/ssd/projects/chromium/src/tools/metrics/histograms/update_histogram_enum.py", line 121, in UpdateHistogramDefinitions
for value, label in source_enum_values.iteritems():
AttributeError: 'tuple' object has no attribute 'iteritems'
I get this error both on a Linux workstation and a Mac laptop.
If I revert https://codereview.chromium.org/2797043002 locally, the script runs, but produces broken results (see attached file). Perhaps this is related to the blink rename. It tries to add a k prefix to most existing entries, but it also tries to add bogus entries such as "void", "static" and "DECLARE_VIRTUAL_TRACE".
Finally, the presubmit check gives the following incorrect warning message about a duplicate entry:
UseCounter::Feature has been updated and there exists duplicated values between (1814) and (kV8HTMLVideoElement_Poster_AttributeGetter)
third_party/WebKit/Source/core/frame/UseCounter.h
,
Apr 13 2017
This might be a dup of bug 711148 , filed last night.
,
Apr 13 2017
,
Apr 13 2017
https://codereview.chromium.org/2815773004/ for a quick fix. I will address a few other concerns in a separate CL.
,
Apr 18 2017
Regarding the k prefix being added, this was caused by the great reformat. dcheng@, can you PTAL? The addition of the k in UseCounter.h has made it more difficult to copy and grep for a use counter, so if we could just remove them again that'd be a quick fix.
,
Apr 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37ab82a26af3aa8755c1ab245240ce92e6b9896c commit 37ab82a26af3aa8755c1ab245240ce92e6b9896c Author: foolip <foolip@chromium.org> Date: Tue Apr 18 09:35:58 2017 Make a use counter name 1 char shorter to avoid line wrapping This line became wrapped in The Blink Rename: https://chromium-review.googlesource.com/472192 Use the DOM spelling of cancelable to get 1 char back: https://dom.spec.whatwg.org/#dom-event-cancelable This should avoid the need to make update_use_counter_feature_enum.py more clever about parsing. BUG= 711205 R=haraken@chromium.org NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2825703002 Cr-Commit-Position: refs/heads/master@{#465189} [modify] https://crrev.com/37ab82a26af3aa8755c1ab245240ce92e6b9896c/third_party/WebKit/Source/core/events/TouchEvent.cpp [modify] https://crrev.com/37ab82a26af3aa8755c1ab245240ce92e6b9896c/third_party/WebKit/Source/core/events/TouchEventTest.cpp [modify] https://crrev.com/37ab82a26af3aa8755c1ab245240ce92e6b9896c/third_party/WebKit/Source/core/frame/UseCounter.h [modify] https://crrev.com/37ab82a26af3aa8755c1ab245240ce92e6b9896c/tools/metrics/histograms/histograms.xml
,
Apr 18 2017
,
Apr 18 2017
https://codereview.chromium.org/2825573003/ likely fixed most or all of this bug.
,
Apr 21 2017
https://codereview.chromium.org/2835733002/ Readdressing other concerns
,
Apr 25 2017
,
May 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/647ac075be56779f27835fc08fd68de07c36d07c commit 647ac075be56779f27835fc08fd68de07c36d07c Author: lunalu <lunalu@chromium.org> Date: Wed May 03 22:27:56 2017 Readdress use_counter_feature_enum issue brought in https://codereview.chromium.org/2797043002 Made histogram checker raise exception for duplicated enum values. BUG= 711205 Review-Url: https://codereview.chromium.org/2835733002 Cr-Commit-Position: refs/heads/master@{#469169} [modify] https://crrev.com/647ac075be56779f27835fc08fd68de07c36d07c/tools/metrics/histograms/update_histogram_enum.py
,
May 4 2017
,
May 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b76b83389ebeb5b4e7a84c7bf7fbcab1bbf92a29 commit b76b83389ebeb5b4e7a84c7bf7fbcab1bbf92a29 Author: fmeawad <fmeawad@chromium.org> Date: Fri May 05 00:07:07 2017 Minor fix UpdateHistogramEnum In https://codereview.chromium.org/2835733002 the ReadHistogramValues method now returns one value instead of 2, but its call in UpdateHistogramEnum still expect 2 where the 2nd was ignored. BUG= 711205 Review-Url: https://codereview.chromium.org/2866573002 Cr-Commit-Position: refs/heads/master@{#469532} [modify] https://crrev.com/b76b83389ebeb5b4e7a84c7bf7fbcab1bbf92a29/tools/metrics/histograms/update_histogram_enum.py
,
Jul 18 2017
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by lunalu@chromium.org
, Apr 13 2017Status: Assigned (was: Untriaged)