New issue
Advanced search Search tips

Issue 754315 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Extension features should be constant after initialization

Project Member Reported by rdevlin....@chromium.org, Aug 10 2017

Issue description

Extension features shouldn't change during runtime, but we pass around raw pointers to unconst features throughout the code.  We should try to make them more constly.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 10 2017

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

commit 8432a8f305c204e5fd87b00351c1921c8eb9b992
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Thu Aug 10 22:24:53 2017

[Extensions][Cleanup] Constify FeatureProvider::GetParent()

Extension Features should not be modified after initialization.
Constify FeatureProvider::GetParent:
- Return a const Feature*.
- Instead of accepting a Feature*, accept a const Feature& (also
  removing a CHECK).

Bug:  754315 
Change-Id: Id90f97408f2464fdbd1c9dc3151d0c0ceb8baf4b
Reviewed-on: https://chromium-review.googlesource.com/610926
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493565}
[modify] https://crrev.com/8432a8f305c204e5fd87b00351c1921c8eb9b992/extensions/common/features/feature_provider.cc
[modify] https://crrev.com/8432a8f305c204e5fd87b00351c1921c8eb9b992/extensions/common/features/feature_provider.h
[modify] https://crrev.com/8432a8f305c204e5fd87b00351c1921c8eb9b992/extensions/renderer/api_definitions_natives.cc
[modify] https://crrev.com/8432a8f305c204e5fd87b00351c1921c8eb9b992/extensions/renderer/feature_cache.cc
[modify] https://crrev.com/8432a8f305c204e5fd87b00351c1921c8eb9b992/extensions/renderer/js_extension_bindings_system.cc
[modify] https://crrev.com/8432a8f305c204e5fd87b00351c1921c8eb9b992/extensions/renderer/native_extension_bindings_system.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 11 2017

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

commit c06f47e34ba7e1922238befb0b022437cb745871
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Aug 11 18:37:57 2017

[Extensions][Cleanup] Constify FeatureProvider::GetFeature and friend

Extension Features should not be modified after initialization.
Constify FeatureProvider::GetFeature() and
ExtensionAPI::GetFeatureDependency().

Bug:  754315 
Change-Id: I2c2da6d62ba387e214c4cdf5e755ecefcfa84f1b
Reviewed-on: https://chromium-review.googlesource.com/611414
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493824}
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/chrome/common/extensions/api/common_extension_api_unittest.cc
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/extensions/common/extension_api.cc
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/extensions/common/extension_api.h
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/extensions/common/features/feature_provider.cc
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/extensions/common/features/feature_provider.h
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/extensions/common/features/feature_provider_unittest.cc
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/extensions/common/features/simple_feature.cc
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/extensions/common/manifest.cc
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/extensions/common/manifest_handlers/permissions_parser.cc
[modify] https://crrev.com/c06f47e34ba7e1922238befb0b022437cb745871/tools/json_schema_compiler/test/features_generation_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 11 2017

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

commit 0221a37ba5a5edc40b0d31d4b1cc2df679ced299
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Aug 11 20:25:51 2017

[Extensions][Cleanup] Constify FeatureProvider::GetChildren

Extension Features should not be modified after initialization.
Constify FeatureProvider::GetChildren() to return a
std::vector<const Feature*> instead of std::vector<Feature*>.

Bug:  754315 
Change-Id: I14a1207f5a25cd27dbdecfd376141bfbc746c261
Reviewed-on: https://chromium-review.googlesource.com/612106
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493841}
[modify] https://crrev.com/0221a37ba5a5edc40b0d31d4b1cc2df679ced299/extensions/common/extension_api.cc
[modify] https://crrev.com/0221a37ba5a5edc40b0d31d4b1cc2df679ced299/extensions/common/features/feature_provider.cc
[modify] https://crrev.com/0221a37ba5a5edc40b0d31d4b1cc2df679ced299/extensions/common/features/feature_provider.h
[modify] https://crrev.com/0221a37ba5a5edc40b0d31d4b1cc2df679ced299/extensions/common/features/feature_provider_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 11 2017

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

commit 9decef811674fa4acc1a58a8d0582ede35deff09
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Aug 11 23:45:57 2017

[Extensions][Cleanup] Constify FeatureProvider feature storage

Extension Features should not be modified after initialization.
Now that all call sites have been updated, we can update the storage in
FeatureProvider to be const from the moment the feature is added. This
should mean features are perma-const once initialized; yay!

Bug:  754315 
Change-Id: If7889a36dc457ad71da014fc275928168f185bc1
Reviewed-on: https://chromium-review.googlesource.com/612126
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493884}
[modify] https://crrev.com/9decef811674fa4acc1a58a8d0582ede35deff09/extensions/common/features/feature_provider.h
[modify] https://crrev.com/9decef811674fa4acc1a58a8d0582ede35deff09/extensions/renderer/feature_cache.cc

Status: Fixed (was: Assigned)

Sign in to add a comment