Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2894063002:
Expose UseCounter::Feature enum out of blink as WebFeature (Closed)
|
Created:
3 years, 7 months ago by lunalu1 Modified:
3 years, 6 months ago CC:
darktears, apavlov+blink_chromium.org, asvitkine+watch_chromium.org, blink-reviews, blink-reviews-api_chromium.org, blink-reviews-bindings_chromium.org, blink-reviews-css, blink-reviews-frames_chromium.org, blink-worker-reviews_chromium.org, chromium-reviews, dglazkov+blink, falken+watch_chromium.org, horo+watch_chromium.org, kinuko+worker_chromium.org, kinuko+watch, rwlbuis, shimazu+worker_chromium.org, Yoav Weiss Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionExpose UseCounter::Feature enum out of blink as WebFeature
UseCounter support is moving to the browser process, and so this
enum definition needs to be exposed publicly.
Copied UseCounter::Feature enum definition to public/platform/WebFeature.h
Created overloaded methods for those that take UseCounter::Feature.
Updated templates that generate code that uses UseCounter::Feature to use
WebFeature instead.
Updated file pass for the enum in presubmit checker.
Future CLs will update all the callers and remove the old definition.
BUG=724287
Review-Url: https://codereview.chromium.org/2894063002
Cr-Commit-Position: refs/heads/master@{#475942}
Committed: https://chromium.googlesource.com/chromium/src/+/addca576a47a9eefc65f1c32ce783ff5cb24bf98
Patch Set 1 : Initial check in #
Total comments: 23
Patch Set 2 : Codereview #Patch Set 3 : Rebase #
Total comments: 1
Patch Set 4 : Codereview: revert change in Histogram.h #Patch Set 5 : Rebase #
Total comments: 4
Patch Set 6 : Codereview (added WATCHLIST and renamed UseCounterDeprecatedFeature.h) + rebase #Patch Set 7 : q #Patch Set 8 : Move enum Feature definition to UseCounterFeature.def and include it in WebFeature and UseCounter::… #Patch Set 9 : Update source path for UseCounter Feature in presubmit checkers #Patch Set 10 : Rebase #Patch Set 11 : Rebase update #Messages
Total messages: 92 (60 generated)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
lunalu@chromium.org changed reviewers: + rbyers@chromium.org
Hi Rick, please take a look.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Description was changed from ========== Copy UseCounter::Feature to WebFeature and mark as deprecated. Copied UseCounter::Feature enum definition to public/platform/WebFeature.h Created overloaded methods for those that take UseCounter::Feature. Updated templates that generate code that uses UseCounter::Feature to use WebFeature instead. Updated file pass for the enum in presubmit checker. BUG=724287 ========== to ========== Expose UseCounter::Feature enum out of blink as WebFeature UseCounter support is moving to the browser process, and so this enum definition needs to be exposed publicly. Copied UseCounter::Feature enum definition to public/platform/WebFeature.h Created overloaded methods for those that take UseCounter::Feature. Updated templates that generate code that uses UseCounter::Feature to use WebFeature instead. Updated file pass for the enum in presubmit checker. Future CLs will update all the callers and remove the old definition. BUG=724287 ==========
Thanks Luna. Big picture this seems like a good plan. The main problem I see is that it's currently an odd cut for what to convert vs. what to add redundant overloads for. I think you can either make this CL a bunch smaller (and follow-on with a later CL - eg. with all the worker changes), or a little bigger (completing all the special cases like workers now while avoiding the complexity of overloads). Let's chat... https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/parser/CSSParserContext.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/parser/CSSParserContext.h:89: return Count(static_cast<UseCounter::Feature>(feature)); I think this would be slightly cleaner if you inverted the relationship here. I.e. update the implementation of 'Count' to use WebFeature, and have the UseCounter::Feature one delegate to the WebFeature overload. This is probably also technically more correct - as some new values may get added to WebFeature before UseCounter::Feature is removed, so WebFeature will be a strict superset of the values in UseCounter::Feature. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/frame/Deprecation.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/Deprecation.h:52: static_cast<UseCounter::Feature>(feature)); ditto about preferring casting from the legacy type to the new type instead of going the other direction. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/frame/UseCounter.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:98: return Count(frame, static_cast<Feature>(feature)); again seems better to update UseCounter.cpp for the new type and do the casts the other direction. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:128: // TODO(lunalu): Deprecate Feature by WebFeature in IsCounted(). Since this is only for testing I think it should be easy to remove the 'Feature' version and keep only 'WebFeature'. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:157: void RecordMeasurement(Feature); I thought this method (and HasRecordedMeasurement) was effectively private (maybe called only by Deprecation.cpp)? Perhaps you can easily keep only the WebFeature version? https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:177: // TODO(lunalu): Deprecate Feature by WebFeature in NotifyFeatureCounted(). I think the Observer infrastructure is also rarely used - again should be easy to just update this now and avoid having the legacy 'Feature' overload here. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:211: // TODO(lunalu): Deprecate Feature by WebFeature in CountFeature(). Definitely shouldn't need the 'Feature' version here - this is all an implementation detail of the UseCounter class. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/BaseFetchContext.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/BaseFetchContext.h:75: // TODO(lunalu): Deprecate UseCounter::Feature by WebFeature in From a quick scan I think all the worker-related support could also be switched over to WebFeature without having to update a ton of callsites. The duplicate call-site thing is really only helpful in the cases where there are hundreds of callers. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... File third_party/WebKit/public/platform/PRESUBMIT.py (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/PRESUBMIT.py:12: def _RunUmaHistogramChecks(input_api, output_api): # pylint: disable=C0103 Looks like this just moves the existing rule without changing it at all except the filename, right? If you haven't alread, please test adding a new WebFeature value to make double sure this is working before you land it. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... File third_party/WebKit/public/platform/WebFeature.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebFeature.h:8: // For now feature IDs are opaque integers (just blink's UseCounter::Feature nit: remove this sentence - with this definition they're no longer "opaque", it's now very precisely defined here. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebFeature.h:12: // TODO(rbyers): Add CSS and animated CSS feature types (eg. by stealing I think we agreed that in your new formulation, the "sealing the top couple bits" choice isn't really an option anymore (we probably need a "tagged-union" data tyle - i.e. type/value tuple). Leaving just "eg. by making this a more sophisticated class" should cover it. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebFeature.h:14: enum class WebFeature : uint32_t { Is there some reason to specify the uin32_t here? It doesn't really matter if it's signed or unsigned, does it?
Oh and I tweaked your CL description a bit to clarify the intent here, I hope you don't mind.
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Hi Rick, I keep getting patch failures due to new features added. Could you please take a look at it first while the bots trying to compile/test. I will try to land this on the weekend when fewer people are trying to touch the features. And I will double check that before I land the CL all features are migrated as wanted. Thanks https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/css/parser/CSSParserContext.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/css/parser/CSSParserContext.h:89: return Count(static_cast<UseCounter::Feature>(feature)); On 2017/05/23 at 19:51:49, Rick Byers wrote: > I think this would be slightly cleaner if you inverted the relationship here. I.e. update the implementation of 'Count' to use WebFeature, and have the UseCounter::Feature one delegate to the WebFeature overload. > > This is probably also technically more correct - as some new values may get added to WebFeature before UseCounter::Feature is removed, so WebFeature will be a strict superset of the values in UseCounter::Feature. Right. Reverted. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/frame/UseCounter.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:98: return Count(frame, static_cast<Feature>(feature)); On 2017/05/23 at 19:51:49, Rick Byers wrote: > again seems better to update UseCounter.cpp for the new type and do the casts the other direction. Done. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:128: // TODO(lunalu): Deprecate Feature by WebFeature in IsCounted(). On 2017/05/23 at 19:51:49, Rick Byers wrote: > Since this is only for testing I think it should be easy to remove the 'Feature' version and keep only 'WebFeature'. OK https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:157: void RecordMeasurement(Feature); On 2017/05/23 at 19:51:49, Rick Byers wrote: > I thought this method (and HasRecordedMeasurement) was effectively private (maybe called only by Deprecation.cpp)? Perhaps you can easily keep only the WebFeature version? Done Not quite, some other places also calls RecordMeasurement and HasRecordedMeasurement. But I updated the type to be WebFeature and did an overloading method for the old type instead. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:177: // TODO(lunalu): Deprecate Feature by WebFeature in NotifyFeatureCounted(). On 2017/05/23 at 19:51:49, Rick Byers wrote: > I think the Observer infrastructure is also rarely used - again should be easy to just update this now and avoid having the legacy 'Feature' overload here. Done. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/frame/UseCounter.h:211: // TODO(lunalu): Deprecate Feature by WebFeature in CountFeature(). On 2017/05/23 at 19:51:49, Rick Byers wrote: > Definitely shouldn't need the 'Feature' version here - this is all an implementation detail of the UseCounter class. I see. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/BaseFetchContext.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/BaseFetchContext.h:75: // TODO(lunalu): Deprecate UseCounter::Feature by WebFeature in On 2017/05/23 at 19:51:49, Rick Byers wrote: > From a quick scan I think all the worker-related support could also be switched over to WebFeature without having to update a ton of callsites. The duplicate call-site thing is really only helpful in the cases where there are hundreds of callers. Reverted. https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... File third_party/WebKit/public/platform/PRESUBMIT.py (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/PRESUBMIT.py:12: def _RunUmaHistogramChecks(input_api, output_api): # pylint: disable=C0103 On 2017/05/23 at 19:51:49, Rick Byers wrote: > Looks like this just moves the existing rule without changing it at all except the filename, right? If you haven't alread, please test adding a new WebFeature value to make double sure this is working before you land it. I did test that. But thanks for the reminder https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... File third_party/WebKit/public/platform/WebFeature.h (right): https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebFeature.h:8: // For now feature IDs are opaque integers (just blink's UseCounter::Feature On 2017/05/23 at 19:51:49, Rick Byers wrote: > nit: remove this sentence - with this definition they're no longer "opaque", it's now very precisely defined here. Done https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebFeature.h:12: // TODO(rbyers): Add CSS and animated CSS feature types (eg. by stealing On 2017/05/23 at 19:51:49, Rick Byers wrote: > I think we agreed that in your new formulation, the "sealing the top couple bits" choice isn't really an option anymore (we probably need a "tagged-union" data tyle - i.e. type/value tuple). Leaving just "eg. by making this a more sophisticated class" should cover it. Done https://codereview.chromium.org/2894063002/diff/20001/third_party/WebKit/publ... third_party/WebKit/public/platform/WebFeature.h:14: enum class WebFeature : uint32_t { On 2017/05/23 at 19:51:49, Rick Byers wrote: > Is there some reason to specify the uin32_t here? It doesn't really matter if it's signed or unsigned, does it? I suppose not, it is just that previously UseCounter::Feature used uint32_t so I thought we should keep it the way it is?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
This looks much cleaner now, thanks! Just one issue. https://codereview.chromium.org/2894063002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/Histogram.h (right): https://codereview.chromium.org/2894063002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/Histogram.h:30: return Count(static_cast<base::HistogramBase::Sample>(feature)); This class is intended to be a generic part of the UMA infrastructure, not refer to any specific histogram enum types. The call sites are just in UseCounter.cpp, right? That code should be responsible for converting values into "Sample"s.
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Done. Thanks. PTAL.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Sorry, I forgot to take out an include in Histogram.h. Uploading another patch.
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Patchset #4 (id:80001) has been deleted
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Hi Rick, could you PTAL? Thanks
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM
The CQ bit was checked by lunalu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rbyers@chromium.org Link to the patchset: https://codereview.chromium.org/2894063002/#ps120001 (title: "Rebase")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lunalu@chromium.org changed reviewers: + thakis@chromium.org
Hi thakis, could you please take a look at: tools/metrics/histograms/update_use_counter_feature_enum.py Thanks
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
I'm a bit confused about this change; how much that matters depends a bit on how long you think it'll take to convert everything to the new header. But I wonder if there's maybe a way to get to the new place without having this inbetween period where we duplicate that whole enum. https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in (right): https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in:1: // NOTE: This enum is being deprecated. Please add your new Feature enum value Why is this called .in instead of .h? It's just included in one file, it's not used as an .in file with a generator. https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... File third_party/WebKit/public/platform/WebFeature.h (right): https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... third_party/WebKit/public/platform/WebFeature.h:12: enum class WebFeature : uint32_t { Why is this list in two places now? Shouldn't the old file just include this new one instead of duplicating the contents? (The presubmit only covers this file, not the other one.)
On 2017/05/26 20:22:31, Nico wrote: > I'm a bit confused about this change; how much that matters depends a bit on how > long you think it'll take to convert everything to the new header. Yeah this is really just a hack to make it easier to stage the refactoring into multiple CLs. In particular I advised Luna to try to keep the one big CL that updates all the callsites 100% script-generated to make it easier to update and review. So the idea here is to lay the groundwork to prepare for updating the ~100 callsites, then land a script-generated CL that updates all the callsites, then land the final CL(s) to clean up remaining uses of the old type and delete it. > But I wonder if there's maybe a way to get to the new place without having this > in-between period where we duplicate that whole enum. IMHO the only way to do that would be a reviewing and maintenance nightmare - UseCounters are added quite frequently and other related code is constantly churning. > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in > (right): > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in:1: // NOTE: > This enum is being deprecated. Please add your new Feature enum value > Why is this called .in instead of .h? It's just included in one file, it's not > used as an .in file with a generator. > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... > File third_party/WebKit/public/platform/WebFeature.h (right): > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... > third_party/WebKit/public/platform/WebFeature.h:12: enum class WebFeature : > uint32_t { > Why is this list in two places now? Shouldn't the old file just include this new > one instead of duplicating the contents? > > (The presubmit only covers this file, not the other one.)
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #6 (id:140001) has been deleted
Patchset #6 (id:160001) has been deleted
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Hi, Thanks for the comments. I made below changes: renamed UseCounterDeprecatedFeature.in to UseCounterDeprecatedFeature.h and added myself to a watchlist for it. PTAL Thanks, Luna https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in (right): https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in:1: // NOTE: This enum is being deprecated. Please add your new Feature enum value On 2017/05/26 at 20:22:31, Nico wrote: > Why is this called .in instead of .h? It's just included in one file, it's not used as an .in file with a generator. Done. Thanks https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... File third_party/WebKit/public/platform/WebFeature.h (right): https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... third_party/WebKit/public/platform/WebFeature.h:12: enum class WebFeature : uint32_t { On 2017/05/26 at 20:22:31, Nico wrote: > Why is this list in two places now? Shouldn't the old file just include this new one instead of duplicating the contents? > > (The presubmit only covers this file, not the other one.) I could add another presubmit checker. I was hoping chromies who edit the features would be more responsible making sure the new feature is added to both places. I added myself to a WATCHLIST for UseCounterDeprecatedFeature.h. That way I will make sure no one modifies that file in the future.
On 2017/05/29 18:46:33, loonybear wrote: > Hi, > > Thanks for the comments. I made below changes: > renamed UseCounterDeprecatedFeature.in to UseCounterDeprecatedFeature.h and > added myself to a watchlist for it. > > PTAL > > Thanks, > Luna > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in > (right): > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in:1: // NOTE: > This enum is being deprecated. Please add your new Feature enum value > On 2017/05/26 at 20:22:31, Nico wrote: > > Why is this called .in instead of .h? It's just included in one file, it's not > used as an .in file with a generator. > > Done. Thanks > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... > File third_party/WebKit/public/platform/WebFeature.h (right): > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... > third_party/WebKit/public/platform/WebFeature.h:12: enum class WebFeature : > uint32_t { > On 2017/05/26 at 20:22:31, Nico wrote: > > Why is this list in two places now? Shouldn't the old file just include this > new one instead of duplicating the contents? > > > > (The presubmit only covers this file, not the other one.) > > I could add another presubmit checker. I was hoping chromies who edit the > features would be more responsible making sure the new feature is added to both > places. > I added myself to a WATCHLIST for UseCounterDeprecatedFeature.h. That way I will > make sure no one modifies that file in the future. Why can't the new file declare the ebum as a regular enum, and then in the old file just include the new file and say `typedef NewEnum OldEnum;`? Then you don't have any duplication.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/05/29 at 18:56:23, thakis wrote: > On 2017/05/29 18:46:33, loonybear wrote: > > Hi, > > > > Thanks for the comments. I made below changes: > > renamed UseCounterDeprecatedFeature.in to UseCounterDeprecatedFeature.h and > > added myself to a watchlist for it. > > > > PTAL > > > > Thanks, > > Luna > > > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... > > File third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in > > (right): > > > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... > > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in:1: // NOTE: > > This enum is being deprecated. Please add your new Feature enum value > > On 2017/05/26 at 20:22:31, Nico wrote: > > > Why is this called .in instead of .h? It's just included in one file, it's not > > used as an .in file with a generator. > > > > Done. Thanks > > > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... > > File third_party/WebKit/public/platform/WebFeature.h (right): > > > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... > > third_party/WebKit/public/platform/WebFeature.h:12: enum class WebFeature : > > uint32_t { > > On 2017/05/26 at 20:22:31, Nico wrote: > > > Why is this list in two places now? Shouldn't the old file just include this > > new one instead of duplicating the contents? > > > > > > (The presubmit only covers this file, not the other one.) > > > > I could add another presubmit checker. I was hoping chromies who edit the > > features would be more responsible making sure the new feature is added to both > > places. > > I added myself to a WATCHLIST for UseCounterDeprecatedFeature.h. That way I will > > make sure no one modifies that file in the future. > > Why can't the new file declare the ebum as a regular enum, and then in the old file just include the new file and say `typedef NewEnum OldEnum;`? Then you don't have any duplication. I have to use enum class because there are CSSProperty enums defined there which would cause duplicated definitions. typedef NewEnum OldEnum doesn't work because when compile we will have UseCounter::FeatureName being replaced by WebFeature::FeatureName and that is what I am going to update in the next CL.
If there are dupe syms wrt CSS, why don't we have them in-tree atm? If that's not fixable, could you put just the enum body into a temporary .def file and include that in both enum definitions? On May 29, 2017 5:15 PM, <lunalu@chromium.org> wrote: > On 2017/05/29 at 18:56:23, thakis wrote: > > On 2017/05/29 18:46:33, loonybear wrote: > > > Hi, > > > > > > Thanks for the comments. I made below changes: > > > renamed UseCounterDeprecatedFeature.in to > UseCounterDeprecatedFeature.h and > > > added myself to a watchlist for it. > > > > > > PTAL > > > > > > Thanks, > > > Luna > > > > > > > https://codereview.chromium.org/2894063002/diff/120001/ > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in > > > File third_party/WebKit/Source/core/frame/ > UseCounterDeprecatedFeature.in > > > (right): > > > > > > > https://codereview.chromium.org/2894063002/diff/120001/ > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature. > in#newcode1 > > > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in:1: > // > NOTE: > > > This enum is being deprecated. Please add your new Feature enum value > > > On 2017/05/26 at 20:22:31, Nico wrote: > > > > Why is this called .in instead of .h? It's just included in one > file, it's > not > > > used as an .in file with a generator. > > > > > > Done. Thanks > > > > > > > https://codereview.chromium.org/2894063002/diff/120001/ > third_party/WebKit/public/platform/WebFeature.h > > > File third_party/WebKit/public/platform/WebFeature.h (right): > > > > > > > https://codereview.chromium.org/2894063002/diff/120001/ > third_party/WebKit/public/platform/WebFeature.h#newcode12 > > > third_party/WebKit/public/platform/WebFeature.h:12: enum class > WebFeature : > > > uint32_t { > > > On 2017/05/26 at 20:22:31, Nico wrote: > > > > Why is this list in two places now? Shouldn't the old file just > include > this > > > new one instead of duplicating the contents? > > > > > > > > (The presubmit only covers this file, not the other one.) > > > > > > I could add another presubmit checker. I was hoping chromies who edit > the > > > features would be more responsible making sure the new feature is > added to > both > > > places. > > > I added myself to a WATCHLIST for UseCounterDeprecatedFeature.h. That > way I > will > > > make sure no one modifies that file in the future. > > > > Why can't the new file declare the ebum as a regular enum, and then in > the old > file just include the new file and say `typedef NewEnum OldEnum;`? Then you > don't have any duplication. > > I have to use enum class because there are CSSProperty enums defined there > which > would cause duplicated definitions. typedef NewEnum OldEnum doesn't work > because > when compile we will have UseCounter::FeatureName being replaced by > WebFeature::FeatureName and that is what I am going to update in the next > CL. > > https://codereview.chromium.org/2894063002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
If there are dupe syms wrt CSS, why don't we have them in-tree atm? If that's not fixable, could you put just the enum body into a temporary .def file and include that in both enum definitions? On May 29, 2017 5:15 PM, <lunalu@chromium.org> wrote: > On 2017/05/29 at 18:56:23, thakis wrote: > > On 2017/05/29 18:46:33, loonybear wrote: > > > Hi, > > > > > > Thanks for the comments. I made below changes: > > > renamed UseCounterDeprecatedFeature.in to > UseCounterDeprecatedFeature.h and > > > added myself to a watchlist for it. > > > > > > PTAL > > > > > > Thanks, > > > Luna > > > > > > > https://codereview.chromium.org/2894063002/diff/120001/ > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in > > > File third_party/WebKit/Source/core/frame/ > UseCounterDeprecatedFeature.in > > > (right): > > > > > > > https://codereview.chromium.org/2894063002/diff/120001/ > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature. > in#newcode1 > > > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in:1: > // > NOTE: > > > This enum is being deprecated. Please add your new Feature enum value > > > On 2017/05/26 at 20:22:31, Nico wrote: > > > > Why is this called .in instead of .h? It's just included in one > file, it's > not > > > used as an .in file with a generator. > > > > > > Done. Thanks > > > > > > > https://codereview.chromium.org/2894063002/diff/120001/ > third_party/WebKit/public/platform/WebFeature.h > > > File third_party/WebKit/public/platform/WebFeature.h (right): > > > > > > > https://codereview.chromium.org/2894063002/diff/120001/ > third_party/WebKit/public/platform/WebFeature.h#newcode12 > > > third_party/WebKit/public/platform/WebFeature.h:12: enum class > WebFeature : > > > uint32_t { > > > On 2017/05/26 at 20:22:31, Nico wrote: > > > > Why is this list in two places now? Shouldn't the old file just > include > this > > > new one instead of duplicating the contents? > > > > > > > > (The presubmit only covers this file, not the other one.) > > > > > > I could add another presubmit checker. I was hoping chromies who edit > the > > > features would be more responsible making sure the new feature is > added to > both > > > places. > > > I added myself to a WATCHLIST for UseCounterDeprecatedFeature.h. That > way I > will > > > make sure no one modifies that file in the future. > > > > Why can't the new file declare the ebum as a regular enum, and then in > the old > file just include the new file and say `typedef NewEnum OldEnum;`? Then you > don't have any duplication. > > I have to use enum class because there are CSSProperty enums defined there > which > would cause duplicated definitions. typedef NewEnum OldEnum doesn't work > because > when compile we will have UseCounter::FeatureName being replaced by > WebFeature::FeatureName and that is what I am going to update in the next > CL. > > https://codereview.chromium.org/2894063002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Done. Please take a look. Thanks
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/30 at 15:33:35, loonybear wrote: > Done. Please take a look. > > Thanks Sorry. I forgot to update the address in PRESUBMIT checkers. I will upload another patch. Apologies.
On 2017/05/29 18:46:33, loonybear wrote: > Hi, > > Thanks for the comments. I made below changes: > renamed UseCounterDeprecatedFeature.in to UseCounterDeprecatedFeature.h and > added myself to a watchlist for it. > > PTAL > > Thanks, > Luna > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in > (right): > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/frame/UseCounterDeprecatedFeature.in:1: // NOTE: > This enum is being deprecated. Please add your new Feature enum value > On 2017/05/26 at 20:22:31, Nico wrote: > > Why is this called .in instead of .h? It's just included in one file, it's not > used as an .in file with a generator. > > Done. Thanks > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... > File third_party/WebKit/public/platform/WebFeature.h (right): > > https://codereview.chromium.org/2894063002/diff/120001/third_party/WebKit/pub... > third_party/WebKit/public/platform/WebFeature.h:12: enum class WebFeature : > uint32_t { > On 2017/05/26 at 20:22:31, Nico wrote: > > Why is this list in two places now? Shouldn't the old file just include this > new one instead of duplicating the contents? > > > > (The presubmit only covers this file, not the other one.) > > I could add another presubmit checker. I was hoping chromies who edit the > features would be more responsible making sure the new feature is added to both > places. I was thinking that we could just snapshot the old enum - there should be no reason to add to it, it exists only temporarily for the sake of existing code which is about to be updated to the new enum. What's the scenario where some new code might need to refer to the old enum? > I added myself to a WATCHLIST for UseCounterDeprecatedFeature.h. That way I will > make sure no one modifies that file in the future.
The CQ bit was checked by lunalu@chromium.org to run a CQ dry run
Done. Please take another look. Thanks
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was checked by lunalu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rbyers@chromium.org Link to the patchset: https://codereview.chromium.org/2894063002/#ps240001 (title: "Update source path for UseCounter Feature in presubmit checkers")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for third_party/WebKit/Source/core/frame/UseCounter.h:
While running git apply --index -3 -p1;
error: patch failed: third_party/WebKit/Source/core/frame/UseCounter.h:78
Falling back to three-way merge...
Applied patch to 'third_party/WebKit/Source/core/frame/UseCounter.h' with
conflicts.
U third_party/WebKit/Source/core/frame/UseCounter.h
Patch: third_party/WebKit/Source/core/frame/UseCounter.h
Index: third_party/WebKit/Source/core/frame/UseCounter.h
diff --git a/third_party/WebKit/Source/core/frame/UseCounter.h
b/third_party/WebKit/Source/core/frame/UseCounter.h
index
59c431c61c22bcb22d18826c80f0278d3c731bac..f122aeac8a8bea065ee605e38c5c3f26fd0205d2
100644
--- a/third_party/WebKit/Source/core/frame/UseCounter.h
+++ b/third_party/WebKit/Source/core/frame/UseCounter.h
@@ -35,6 +35,7 @@
#include "platform/wtf/BitVector.h"
#include "platform/wtf/Noncopyable.h"
#include "platform/wtf/text/WTFString.h"
+#include "public/platform/WebFeature.h"
#include "v8/include/v8.h"
namespace blink {
@@ -78,1551 +79,7 @@ class CORE_EXPORT UseCounter {
UseCounter(Context = kDefaultContext);
enum Feature : uint32_t {
- // Do not change assigned numbers of existing items: add new features
- // to the end of the list.
- kOBSOLETE_PageDestruction = 0,
- kWorkerStart = 4,
- kSharedWorkerStart = 5,
- kUnprefixedIndexedDB = 9,
- kOpenWebDatabase = 10,
- kUnprefixedRequestAnimationFrame = 13,
- kPrefixedRequestAnimationFrame = 14,
- kContentSecurityPolicy = 15,
- kContentSecurityPolicyReportOnly = 16,
- kPrefixedTransitionEndEvent = 18,
- kUnprefixedTransitionEndEvent = 19,
- kPrefixedAndUnprefixedTransitionEndEvent = 20,
- kAutoFocusAttribute = 21,
- kDataListElement = 23,
- kFormAttribute = 24,
- kIncrementalAttribute = 25,
- kInputTypeColor = 26,
- kInputTypeDate = 27,
- kInputTypeDateTimeFallback = 29,
- kInputTypeDateTimeLocal = 30,
- kInputTypeEmail = 31,
- kInputTypeMonth = 32,
- kInputTypeNumber = 33,
- kInputTypeRange = 34,
- kInputTypeSearch = 35,
- kInputTypeTel = 36,
- kInputTypeTime = 37,
- kInputTypeURL = 38,
- kInputTypeWeek = 39,
- kInputTypeWeekFallback = 40,
- kListAttribute = 41,
- kMaxAttribute = 42,
- kMinAttribute = 43,
- kPatternAttribute = 44,
- kPlaceholderAttribute = 45,
- kPrefixedDirectoryAttribute = 47,
- kRequiredAttribute = 49,
- kStepAttribute = 51,
- kPageVisits = 52,
- kHTMLMarqueeElement = 53,
- kReflection = 55,
- kPrefixedStorageInfo = 57,
- kDeprecatedFlexboxWebContent = 61,
- kDeprecatedFlexboxChrome = 62,
- kDeprecatedFlexboxChromeExtension = 63,
- kUnprefixedPerformanceTimeline = 65,
- kUnprefixedUserTiming = 67,
- kWindowEvent = 69,
- kContentSecurityPolicyWithBaseElement = 70,
- kDocumentClear = 74,
- kXMLDocument = 77,
- kXSLProcessingInstruction = 78,
- kXSLTProcessor = 79,
- kSVGSwitchElement = 80,
- kDocumentAll = 83,
- kFormElement = 84,
- kDemotedFormElement = 85,
- kSVGAnimationElement = 90,
- kLineClamp = 96,
- kSubFrameBeforeUnloadRegistered = 97,
- kSubFrameBeforeUnloadFired = 98,
- kConsoleMarkTimeline = 102,
- kDocumentCreateAttribute = 111,
- kDocumentCreateAttributeNS = 112,
- kDocumentXMLEncoding = 115, // Removed from DOM4.
- kDocumentXMLStandalone = 116, // Removed from DOM4.
- kDocumentXMLVersion = 117, // Removed from DOM4.
- kNavigatorProductSub = 123,
- kNavigatorVendor = 124,
- kNavigatorVendorSub = 125,
- kPrefixedAnimationEndEvent = 128,
- kUnprefixedAnimationEndEvent = 129,
- kPrefixedAndUnprefixedAnimationEndEvent = 130,
- kPrefixedAnimationStartEvent = 131,
- kUnprefixedAnimationStartEvent = 132,
- kPrefixedAndUnprefixedAnimationStartEvent = 133,
- kPrefixedAnimationIterationEvent = 134,
- kUnprefixedAnimationIterationEvent = 135,
- kPrefixedAndUnprefixedAnimationIterationEvent = 136,
- kEventReturnValue = 137, // Legacy IE extension.
- kSVGSVGElement = 138,
- kDOMSubtreeModifiedEvent = 143,
- kDOMNodeInsertedEvent = 144,
- kDOMNodeRemovedEvent = 145,
- kDOMNodeRemovedFromDocumentEvent = 146,
- kDOMNodeInsertedIntoDocumentEvent = 147,
- kDOMCharacterDataModifiedEvent = 148,
- kDocumentAllLegacyCall = 150,
- kGetMatchedCSSRules = 155,
- kPrefixedAudioDecodedByteCount = 164,
- kPrefixedVideoDecodedByteCount = 165,
- kPrefixedVideoSupportsFullscreen = 166,
- kPrefixedVideoDisplayingFullscreen = 167,
- kPrefixedVideoEnterFullscreen = 168,
- kPrefixedVideoExitFullscreen = 169,
- kPrefixedVideoEnterFullScreen = 170,
- kPrefixedVideoExitFullScreen = 171,
- kPrefixedVideoDecodedFrameCount = 172,
- kPrefixedVideoDroppedFrameCount = 173,
- kPrefixedElementRequestFullscreen = 176,
- kPrefixedElementRequestFullScreen = 177,
- kBarPropLocationbar = 178,
- kBarPropMenubar = 179,
- kBarPropPersonalbar = 180,
- kBarPropScrollbars = 181,
- kBarPropStatusbar = 182,
- kBarPropToolbar = 183,
- kInputTypeEmailMultiple = 184,
- kInputTypeEmailMaxLength = 185,
- kInputTypeEmailMultipleMaxLength = 186,
- kInputTypeText = 190,
- kInputTypeTextMaxLength = 191,
- kInputTypePassword = 192,
- kInputTypePasswordMaxLength = 193,
- kPrefixedPageVisibility = 196,
- kDocumentBeforeUnloadRegistered = 200,
- kDocumentBeforeUnloadFired = 201,
- kDocumentUnloadRegistered = 202,
- kDocumentUnloadFired = 203,
- kSVGLocatableNearestViewportElement = 204,
- kSVGLocatableFarthestViewportElement = 205,
- kSVGPointMatrixTransform = 209,
- kDOMFocusInOutEvent = 211,
- kFileGetLastModifiedDate = 212,
- kHTMLElementInnerText = 213,
- kHTMLElementOuterText = 214,
- kReplaceDocumentViaJavaScriptURL = 215,
- kElementPrefixedMatchesSelector = 217,
- kCSSStyleSheetRules = 219,
- kCSSStyleSheetAddRule = 220,
- kCSSStyleSheetRemoveRule = 221,
- // The above items are available in M33 branch.
-
- kInitMessageEvent = 222,
- kPrefixedDevicePixelRatioMediaFeature = 233,
- kPrefixedMaxDevicePixelRatioMediaFeature = 234,
- kPrefixedMinDevicePixelRatioMediaFeature = 235,
- kPrefixedTransform3dMediaFeature = 237,
- kPrefixedStorageQuota = 240,
- kResetReferrerPolicy = 243,
- // Case-insensitivity dropped from specification.
- kCaseInsensitiveAttrSelectorMatch = 244,
- kFormNameAccessForImageElement = 246,
- kFormNameAccessForPastNamesMap = 247,
- kFormAssociationByParser = 248,
- kSVGSVGElementInDocument = 250,
- kSVGDocumentRootElement = 251,
- kWorkerSubjectToCSP = 257,
- kWorkerAllowedByChildBlockedByScript = 258,
- kDeprecatedWebKitGradient = 260,
- kDeprecatedWebKitLinearGradient = 261,
- kDeprecatedWebKitRepeatingLinearGradient = 262,
- kDeprecatedWebKitRadialGradient = 263,
- kDeprecatedWebKitRepeatingRadialGradient = 264,
- // The above items are available in M34 branch.
-
- kTextAutosizing = 274,
- kHTMLAnchorElementPingAttribute = 276,
- kSVGClassName = 279,
- kHTMLMediaElementSeekToFragmentStart = 281,
- kHTMLMediaElementPauseAtFragmentEnd = 282,
- kPrefixedWindowURL = 283,
- kWindowOrientation = 285,
- kDocumentCaptureEvents = 287,
- kDocumentReleaseEvents = 288,
- kWindowCaptureEvents = 289,
- kWindowReleaseEvents = 290,
- kDocumentXPathCreateExpression = 295,
- kDocumentXPathCreateNSResolver = 296,
- kDocumentXPathEvaluate = 297,
- kAnimationConstructorKeyframeListEffectObjectTiming = 300,
- kAnimationConstructorKeyframeListEffectNoTiming = 302,
- kPrefixedCancelAnimationFrame = 304,
- kNamedNodeMapGetNamedItem = 306,
- kNamedNodeMapSetNamedItem = 307,
- kNamedNodeMapRemoveNamedItem = 308,
- kNamedNodeMapItem = 309,
- kNamedNodeMapGetNamedItemNS = 310,
- kNamedNodeMapSetNamedItemNS = 311,
- kNamedNodeMapRemoveNamedItemNS = 312,
- kPrefixedDocumentIsFullscreen = 318,
- kPrefixedDocumentCurrentFullScreenElement = 320,
- kPrefixedDocumentCancelFullScreen = 321,
- kPrefixedDocumentFullscreenEnabled = 322,
- kPrefixedDocumentFullscreenElement = 323,
- kPrefixedDocumentExitFullscreen = 324,
- // The above items are available in M35 branch.
-
- kSVGForeignObjectElement = 325,
- kSelectionSetPosition = 327,
- kAnimationFinishEvent = 328,
- kSVGSVGElementInXMLDocument = 329,
- kEventSrcElement = 343,
- kEventCancelBubble = 344,
- kEventPath = 345,
- kNodeIteratorDetach = 347,
- kEventGetReturnValueTrue = 350,
- kEventGetReturnValueFalse = 351,
- kEventSetReturnValueTrue = 352,
- kEventSetReturnValueFalse = 353,
- kWindowOffscreenBuffering = 356,
- kWindowDefaultStatus = 357,
- kWindowDefaultstatus = 358,
- kPrefixedTransitionEventConstructor = 361,
- kPrefixedMutationObserverConstructor = 362,
- kNotificationPermission = 371,
- kRangeDetach = 372,
- kPrefixedFileRelativePath = 386,
- kDocumentCaretRangeFromPoint = 387,
- kElementScrollIntoViewIfNeeded = 389,
- kRangeExpand = 393,
- kHTMLImageElementX = 396,
- kHTMLImageElementY = 397,
- kSelectionBaseNode = 400,
- kSelectionBaseOffset = 401,
- kSelectionExtentNode = 402,
- kSelectionExtentOffset = 403,
- kSelectionType = 404,
- kSelectionModify = 405,
- kSelectionSetBaseAndExtent = 406,
- kSelectionEmpty = 407,
- kVTTCue = 409,
- kVTTCueRender = 410,
- kVTTCueRenderVertical = 411,
- kVTTCueRenderSnapToLinesFalse = 412,
- kVTTCueRenderLineNotAuto = 413,
- kVTTCueRenderPositionNot50 = 414,
- kVTTCueRenderSizeNot100 = 415,
- kVTTCueRenderAlignNotCenter = 416,
- // The above items are available in M36 branch.
-
- kElementRequestPointerLock = 417,
- kVTTCueRenderRtl = 418,
- kPostMessageFromSecureToInsecure = 419,
- kPostMessageFromInsecureToSecure = 420,
- kDocumentExitPointerLock = 421,
- kDocumentPointerL…
(message too large)
The CQ bit was checked by lunalu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org, rbyers@chromium.org Link to the patchset: https://codereview.chromium.org/2894063002/#ps260001 (title: "Rebase")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) linux_chromium_headless_rel on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #10 (id:260001) has been deleted
The CQ bit was checked by lunalu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org, rbyers@chromium.org Link to the patchset: https://codereview.chromium.org/2894063002/#ps280001 (title: "Rebase")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by lunalu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org, rbyers@chromium.org Link to the patchset: https://codereview.chromium.org/2894063002/#ps300001 (title: "Rebase update")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 300001, "attempt_start_ts": 1496241747209770,
"parent_rev": "1084d3e3db822217e7a1c00deebc0b66738085c3", "commit_rev":
"addca576a47a9eefc65f1c32ce783ff5cb24bf98"}
Message was sent while issue was closed.
Description was changed from ========== Expose UseCounter::Feature enum out of blink as WebFeature UseCounter support is moving to the browser process, and so this enum definition needs to be exposed publicly. Copied UseCounter::Feature enum definition to public/platform/WebFeature.h Created overloaded methods for those that take UseCounter::Feature. Updated templates that generate code that uses UseCounter::Feature to use WebFeature instead. Updated file pass for the enum in presubmit checker. Future CLs will update all the callers and remove the old definition. BUG=724287 ========== to ========== Expose UseCounter::Feature enum out of blink as WebFeature UseCounter support is moving to the browser process, and so this enum definition needs to be exposed publicly. Copied UseCounter::Feature enum definition to public/platform/WebFeature.h Created overloaded methods for those that take UseCounter::Feature. Updated templates that generate code that uses UseCounter::Feature to use WebFeature instead. Updated file pass for the enum in presubmit checker. Future CLs will update all the callers and remove the old definition. BUG=724287 Review-Url: https://codereview.chromium.org/2894063002 Cr-Commit-Position: refs/heads/master@{#475942} Committed: https://chromium.googlesource.com/chromium/src/+/addca576a47a9eefc65f1c32ce78... ==========
Message was sent while issue was closed.
Committed patchset #11 (id:300001) as https://chromium.googlesource.com/chromium/src/+/addca576a47a9eefc65f1c32ce78... |
