New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 756488 link

Starred by 1 user
Status: Fixed
Owner:
rdevlin....@chromium.org
Closed: Aug 2017
Cc:
karandeepb@chromium.org
█ catmulli...@chromium.org
lazyboy@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Expand ExtensionBuilder to be more useful and semantic

Project Member Reported by rdevlin....@chromium.org, Aug 17 2017 
ExtensionBuilder is a handy utility for constructing extensions in a (usually unit) test environment, since Extension::Create() is a pain. However, it only provides a SetManifest() method, which means that tests still need to construct the manifest more-or-less by hand.

We should expand ExtensionBuilder to be able to make the creation process more streamlined by adding logic for constructing simple extensions and common extension traits like permissions or actions.

Ideally, this will also help us get rid of the <n> different test extension construction methods and consolidate (most of) them to one canonical one.  (or, maybe this happens: https://xkcd.com/927/ - but hopefully not!)
Project Member

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

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

commit b8e65cd0c063ca2ff9f844513292ddfa0f092334
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Aug 18 20:36:09 2017

[Extensions] Expand ExtensionBuilder to be more useful and semantic

ExtensionBuilder is a handy utility for constructing extensions in a
(usually unit) test environment, since Extension::Create() is a pain.
However, it only provides a SetManifest() method, which means that tests
still need to construct the manifest more-or-less by hand.

Expand ExtensionBuilder to be able to make the creation process more
streamlined by adding logic for constructing simple extensions. Also add
utility methods for common customizations, such as adding permissions.

Use the new builder in two places in the renderer to demonstrate its
functionality.

Also add some basic unit tests for ExtensionBuilder.

Before:
  DictionaryBuilder manifest;
  manifest.Set("name", "ext")
          .Set("manifest_version", 2)
          .Set("version", "0.1")
          .Set("description", "some extension")
          .Set("permissions", ListBuilder().Append("storage").Build());
  DictionaryBuilder background;
  background.Set("scripts", ListBuilder().Append("test.js").Build());
  manifest.Set(
      "app",
      DictionaryBuilder().Set("background", background.Build()).Build());
  scoped_refptr<const Extension> extension =
      ExtensionBuilder().SetManifest(manifest.Build())
                        .SetID(crx_file::id_util::GenerateId("ext"))
                        .Build();

After:
  scoped_refptr<const Extension> extension =
      ExtensionBuilder("ext", ExtensionBuilder::Type::PLATFORM_APP)
          .AddPermission("storage")
          .Build();

Bug:  756488 
Change-Id: I58088afcf25f2d9bb37948a331c9801d7b3122b0
Reviewed-on: https://chromium-review.googlesource.com/619270
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495672}
[modify] https://crrev.com/b8e65cd0c063ca2ff9f844513292ddfa0f092334/extensions/common/BUILD.gn
[modify] https://crrev.com/b8e65cd0c063ca2ff9f844513292ddfa0f092334/extensions/common/extension_builder.cc
[modify] https://crrev.com/b8e65cd0c063ca2ff9f844513292ddfa0f092334/extensions/common/extension_builder.h
[add] https://crrev.com/b8e65cd0c063ca2ff9f844513292ddfa0f092334/extensions/common/extension_builder_unittest.cc
[modify] https://crrev.com/b8e65cd0c063ca2ff9f844513292ddfa0f092334/extensions/renderer/feature_cache_unittest.cc
[modify] https://crrev.com/b8e65cd0c063ca2ff9f844513292ddfa0f092334/extensions/renderer/native_extension_bindings_system_unittest.cc
Project Member

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

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

commit 05b047de40f76ea7a0752deefe99786bd5da46bd
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Aug 22 21:53:55 2017

[Extensions Cleanup] Use the semantic methods of ExtensionBuilder

Update extension_service_unittest to use the semantic methods of
ExtensionBuilder.

Bug:  756488 

Change-Id: I743c26cd8ec0199835a3ce5f3b2d3bdc22455c1b
Reviewed-on: https://chromium-review.googlesource.com/626876
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496459}
[modify] https://crrev.com/05b047de40f76ea7a0752deefe99786bd5da46bd/chrome/browser/extensions/extension_service_unittest.cc
Project Member

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

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

commit 3f5b18f3a5491efc33c821d661131bc417e51698
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Aug 22 21:54:05 2017

[Extensions Cleanup] Use the semantic methods of ExtensionBuilder

Update extension_service_sync_unittest to use the semantic methods of
ExtensionBuilder.

Bug:  756488 
Change-Id: Ie97834a2c01d1e7b2a3bbd04c5417de943f561a2
Reviewed-on: https://chromium-review.googlesource.com/626878
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496460}
[modify] https://crrev.com/3f5b18f3a5491efc33c821d661131bc417e51698/chrome/browser/extensions/extension_service_sync_unittest.cc

Comment 4 by rdevlin....@chromium.org, Aug 25 2017

Status: Fixed (was: Started)
There's lots of migration left, but I think the core of this bug is fixed.  The rest can be migrated on an as-needed basis.  Closing this one out.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 15 2017 

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

commit 124dcd8039c60057b98bade99b76546b4a44bee5
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Sep 15 18:58:33 2017

[Extensions] Replace extension_action_test_util::CreateActionExtension

extension_action_test_util::CreateActionExtension can now be replaced
with ExtensionBuilder. Do so, and remove one more of the extraneous ways
to create a test extension.

Bug:  756488 
Change-Id: I44b99632e26883c677b8344894f7e072174bb7b0
Reviewed-on: https://chromium-review.googlesource.com/622412
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502322}
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/extensions/extension_action_test_util.cc
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/extensions/extension_action_test_util.h
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/cocoa/extensions/browser_action_button_interactive_uitest.mm
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/cocoa/toolbar/toolbar_controller_unittest.mm
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/extensions/extension_action_view_controller_unittest.cc
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/extensions/extension_installed_bubble_browsertest.cc
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/extensions/extension_message_bubble_browsertest.cc
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.h
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/toolbar/toolbar_actions_model_unittest.cc
[modify] https://crrev.com/124dcd8039c60057b98bade99b76546b4a44bee5/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 2 2017 

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

commit f9f764f376d6e6e4ea9aff27d454c4b10b91cfe4
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Mon Oct 02 18:25:45 2017

[Extensions] Use semantic ExtensionBuilder in features unittests

Use the cleaner, semantic ExtensionBuilder in
feature_provider_unittest.cc to cut down on a bit of clutter.

Bug:  756488 
Change-Id: Ic1b7ad5adadd2f05679ada5860cec0c8d916075e
Reviewed-on: https://chromium-review.googlesource.com/693614
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505682}
[modify] https://crrev.com/f9f764f376d6e6e4ea9aff27d454c4b10b91cfe4/extensions/common/features/feature_provider_unittest.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 12 2018 

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

commit cbc15a3a0c4aadfa1277f2fbd25eb35590ba1708
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Thu Apr 12 09:28:39 2018

[Extensions] Update some extensions browser tests to use manifest v2

Some extension browser tests generate extension objects at runtime.
Update these to generate extensions with manifest_version: 2. Where it
wasn't already used, also update these to use ExtensionBuilder, which
uses manifest v2 by default.

Bug:  816679 
Bug:  756488 
Change-Id: I7142d085ad9569dccd4e9b62e09c7a68da08e360
Reviewed-on: https://chromium-review.googlesource.com/1008633
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550113}
[modify] https://crrev.com/cbc15a3a0c4aadfa1277f2fbd25eb35590ba1708/chrome/browser/extensions/api/sessions/sessions_apitest.cc
[modify] https://crrev.com/cbc15a3a0c4aadfa1277f2fbd25eb35590ba1708/chrome/browser/extensions/extension_install_prompt_browsertest.cc
[modify] https://crrev.com/cbc15a3a0c4aadfa1277f2fbd25eb35590ba1708/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 12 2018 

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

commit 1fa235b63f813db3edd77307d6c85c37ddb65606
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Thu Apr 12 14:04:07 2018

[Extensions] Update extensions unittests to use manifest v2

A bunch of extension unittests generate extension objects at runtime.
Update these to generate extensions with manifest_version: 2. Where
trivial, update these to use ExtensionBuilder (if they weren't already),
which uses manifest v2 by default.

Bug:  816679 
Bug:  756488 

Change-Id: Iec80a5dd288452e70e5dbdd1902180611f6bbcca
Reviewed-on: https://chromium-review.googlesource.com/1007501
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550180}
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_management_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_prefs_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_protocols_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_web_ui_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/install_tracker_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/permissions_based_management_policy_provider_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/standard_management_policy_provider_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/test_extension_prefs.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/updater/extension_updater_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/common/extensions/manifest_handlers/settings_overrides_handler_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/common/extensions/manifest_handlers/ui_overrides_handler_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/common/extensions/permissions/permission_set_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/common/extensions/sync_type_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/test/data/extensions/manifest_tests/background_scripts.json
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/test/data/extensions/page_action/page_action.json
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/test/data/extensions/page_action/page_action_invalid_title.json
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api/declarative/rules_registry_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api/power/power_api_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api/storage/settings_test_util.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api_test_utils.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/app_window/app_window_geometry_cache_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/extension_registry_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/info_map_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/policy_check_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/requirements_checker_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/runtime_data_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/common/api/declarative/declarative_manifest_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/common/extension_set_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/common/file_util_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/shell/browser/api/identity/identity_api_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/shell/browser/system_logs/shell_system_logs_fetcher_unittest.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1fa235b63f813db3edd77307d6c85c37ddb65606

commit 1fa235b63f813db3edd77307d6c85c37ddb65606
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Thu Apr 12 14:04:07 2018

[Extensions] Update extensions unittests to use manifest v2

A bunch of extension unittests generate extension objects at runtime.
Update these to generate extensions with manifest_version: 2. Where
trivial, update these to use ExtensionBuilder (if they weren't already),
which uses manifest v2 by default.

Bug:  816679 
Bug:  756488 

Change-Id: Iec80a5dd288452e70e5dbdd1902180611f6bbcca
Reviewed-on: https://chromium-review.googlesource.com/1007501
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550180}
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_gcm_app_handler_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_management_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_prefs_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_protocols_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/extension_web_ui_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/install_tracker_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/permissions_based_management_policy_provider_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/standard_management_policy_provider_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/test_extension_prefs.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/browser/extensions/updater/extension_updater_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/common/extensions/manifest_handlers/settings_overrides_handler_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/common/extensions/manifest_handlers/ui_overrides_handler_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/common/extensions/permissions/permission_set_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/common/extensions/sync_type_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/test/data/extensions/manifest_tests/background_scripts.json
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/test/data/extensions/page_action/page_action.json
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/chrome/test/data/extensions/page_action/page_action_invalid_title.json
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api/declarative/rules_registry_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api/power/power_api_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api/storage/settings_test_util.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api_test_utils.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/api_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/app_window/app_window_geometry_cache_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/extension_registry_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/info_map_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/policy_check_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/requirements_checker_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/browser/runtime_data_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/common/api/declarative/declarative_manifest_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/common/extension_set_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/common/file_util_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/shell/browser/api/identity/identity_api_unittest.cc
[modify] https://crrev.com/1fa235b63f813db3edd77307d6c85c37ddb65606/extensions/shell/browser/system_logs/shell_system_logs_fetcher_unittest.cc
►

Sign in to add a comment