New issue
Advanced search Search tips

Issue 832958 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug


Show other hotlists

Hotlists containing this issue:
Hotlist-1


Sign in to add a comment

Semantic ExtensionBuilder v2

Project Member Reported by rdevlin....@chromium.org, Apr 13 2018

Issue description

We can expand ExtensionBuilder a bit more to make it even more useful.

Proposed changes:
- ExtensionBuilder::SetBackgroundPage()
- ExtensionBuilder::SetManifestValue()
- ExtensionBuilder::SetVersion()
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 16 2018

Project Member

Comment 2 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/+/e45791c333e37d708f566cefa072cdea77116778

commit e45791c333e37d708f566cefa072cdea77116778
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Mon Apr 16 16:02:26 2018

[Extensions] Add ExtensionBuilder::SetBackgroundPage()

Setting a background page is a pretty common operation in test
extensions. Bake it into ExtensionBuilder.

Use it in a small smattering of places, and add a unittest.

Bug: 832958
Change-Id: Iaa3464dfa183f990fb579d289f99271983344983
Reviewed-on: https://chromium-review.googlesource.com/1013128
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550982}
[modify] https://crrev.com/e45791c333e37d708f566cefa072cdea77116778/chrome/renderer/extensions/extension_hooks_delegate_unittest.cc
[modify] https://crrev.com/e45791c333e37d708f566cefa072cdea77116778/extensions/browser/lazy_background_task_queue_unittest.cc
[modify] https://crrev.com/e45791c333e37d708f566cefa072cdea77116778/extensions/browser/runtime_data_unittest.cc
[modify] https://crrev.com/e45791c333e37d708f566cefa072cdea77116778/extensions/common/extension_builder.cc
[modify] https://crrev.com/e45791c333e37d708f566cefa072cdea77116778/extensions/common/extension_builder.h
[modify] https://crrev.com/e45791c333e37d708f566cefa072cdea77116778/extensions/common/extension_builder_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 8 2018

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

commit 98cd65857620870d7dc943ee4bc42581ff14b38f
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue May 08 19:18:12 2018

[Extensions] Add ExtensionBuilder::SetManifest[Key|Path]()

Add utility methods to ExtensionBuilder to allow for setting a manifest
key or path without needing to go through MergeManifest(). This saves
the hassle of needing to construct a dictionary with a key in order to
set a manifest value.

Use it in a smattering of places.

Bug: 832958

Change-Id: If885273205572ba72787e21acc1a75da4f861751
Reviewed-on: https://chromium-review.googlesource.com/1048848
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556917}
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/extensions/api/file_system/consent_provider_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/extensions/api/identity/identity_apitest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/extensions/extension_garbage_collector_chromeos_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/extensions/extension_protocols_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/extensions/extension_service_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/extensions/extension_web_ui_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/extensions/install_verifier_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/extensions/permission_messages_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/extensions/browser/updater/update_service_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/extensions/common/extension_builder.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/extensions/common/extension_builder.h
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/extensions/common/extension_builder_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/extensions/renderer/runtime_hooks_delegate_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/extensions/shell/browser/api/identity/identity_api_unittest.cc
[modify] https://crrev.com/98cd65857620870d7dc943ee4bc42581ff14b38f/extensions/shell/browser/system_logs/shell_system_logs_fetcher_unittest.cc

Another proposed change: AddContentScript() (since constructing the content script value is a pain).

I'm on the fence about whether SetVersion() is useful, since SetManifestValue makes it pretty easy.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 15

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

commit 02eead20bf893a9734e906559e52429879bf48c8
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Sat Sep 15 03:49:11 2018

[Extensions] Add ExtensionBuilder::AddContentScript()

Add a method, ExtensionBuilder::AddContentScript(), which adds a
content script entry to the extension with the specified script name
and match patterns. This makes adding content scripts to extensions in
unit tests much simpler, since we don't have to construct the
dictionary values by hand.

Add a unittest for the same. As part of this, we need to move content
script handler registration to the //extensions layer; this probably
should have been done anyway, since it lives in the //extensions layer.

Also use the new method in ScriptingPermissionsModifier unittests and
ExtensionContextMenuModel unittests.

TBR=halliwell@chromium.org (fairly mechanical change in cast_extensions_api_provider.cc)

Bug: 832958
Change-Id: I8ba4b126e44e8a60b913af5f90b63c747a4271c8
Reviewed-on: https://chromium-review.googlesource.com/1226463
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591572}
[modify] https://crrev.com/02eead20bf893a9734e906559e52429879bf48c8/chrome/browser/extensions/extension_context_menu_model_unittest.cc
[modify] https://crrev.com/02eead20bf893a9734e906559e52429879bf48c8/chrome/browser/extensions/scripting_permissions_modifier_unittest.cc
[modify] https://crrev.com/02eead20bf893a9734e906559e52429879bf48c8/chrome/common/extensions/chrome_manifest_handlers.cc
[modify] https://crrev.com/02eead20bf893a9734e906559e52429879bf48c8/chromecast/common/cast_extensions_api_provider.cc
[modify] https://crrev.com/02eead20bf893a9734e906559e52429879bf48c8/extensions/common/common_manifest_handlers.cc
[modify] https://crrev.com/02eead20bf893a9734e906559e52429879bf48c8/extensions/common/extension_builder.cc
[modify] https://crrev.com/02eead20bf893a9734e906559e52429879bf48c8/extensions/common/extension_builder.h
[modify] https://crrev.com/02eead20bf893a9734e906559e52429879bf48c8/extensions/common/extension_builder_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 25

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

commit 5d741ba96e471879d5de3583ef0083094f11b177
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Sep 25 18:37:50 2018

[Extensions] Add ExtensionBuilder::SetVersion()

ExtensionBuilder has SetManifestKey(), but "version" is pretty common
(and fairly core to the extension object), so add a convenience method
for setting it.

Add a quick unittest for the same, and update a smattering of places
that previously used SetManifestKey().

Bug: 832958

Change-Id: I55de79360e698f8f9983c24a0e0d5fdb8b3fff9b
Reviewed-on: https://chromium-review.googlesource.com/1242196
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594023}
[modify] https://crrev.com/5d741ba96e471879d5de3583ef0083094f11b177/chrome/browser/extensions/extension_garbage_collector_chromeos_unittest.cc
[modify] https://crrev.com/5d741ba96e471879d5de3583ef0083094f11b177/extensions/browser/updater/update_service_unittest.cc
[modify] https://crrev.com/5d741ba96e471879d5de3583ef0083094f11b177/extensions/common/extension_builder.cc
[modify] https://crrev.com/5d741ba96e471879d5de3583ef0083094f11b177/extensions/common/extension_builder.h
[modify] https://crrev.com/5d741ba96e471879d5de3583ef0083094f11b177/extensions/common/extension_builder_unittest.cc
[modify] https://crrev.com/5d741ba96e471879d5de3583ef0083094f11b177/extensions/shell/browser/system_logs/shell_system_logs_fetcher_unittest.cc

Sign in to add a comment