New issue
Advanced search Search tips

Issue 893373 link

Starred by 4 users

Issue metadata

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

Blocked on:
issue 893375



Sign in to add a comment

Extensions: Common "action" manifest key

Project Member Reported by rdevlin....@chromium.org, Oct 9

Issue description

We still have "pageAction" and "browserAction", but they are essentially equivalent (with the primary difference being that browser actions are default-enabled and page actions are default-disabled).  We should combine these into a common "action" key.

Original design doc from catmullings@:
https://docs.google.com/document/d/1J6AXI-pkpnSueDH1pHSyj2y_W8Z5dLCFx1Zi9H4Ju_I/edit
 
Blockedon: 893375
Could you please give me access to https://docs.google.com/document/d/1J6AXI-pkpnSueDH1pHSyj2y_W8Z5dLCFx1Zi9H4Ju_I/edit ?
We have feature based on the API and it would be nice to know expected changes in the API.
Unfortunately, that doc is internal and owned by a former team member.  It's also a little out of date in a few areas.  I'll see if I can draft a new, public one and share it here.
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 20

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

commit 0b1122608d92d8eac0fff92a847cd927715d9d32
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Thu Dec 20 03:45:17 2018

[Extensions] Combine a couple action-related browser tests

Expand ExtensionActionAPITest to be a more flexible test suite and be
geared towards running tests on both page and browser actions, when
applicable. This allows us to increase test coverage (in cases where
only one was tested), as well as eliminating duplicate code (in cases
where each was tested separately). This will also be useful in the
introduction of the "action" key, which should behave similarly as well.

In this CL, create a parameterized test case to run with both page and
browser actions, and convert the "title localization" browser test to
use this. This results in significantly less code, as well as fixing a
configuration bug where the browser action browsertest was, in fact, not
being compiled in the binary.

Bug: 893373
Change-Id: If24f6fdc97b7267e0cff4a630e8375efb9220a79
Reviewed-on: https://chromium-review.googlesource.com/c/1378668
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618100}
[modify] https://crrev.com/0b1122608d92d8eac0fff92a847cd927715d9d32/chrome/browser/extensions/api/extension_action/extension_action_apitest.cc
[delete] https://crrev.com/c59c9c43360f9d1467eac55bca76bb88f337979b/chrome/browser/extensions/browser_action_apitest.cc
[modify] https://crrev.com/0b1122608d92d8eac0fff92a847cd927715d9d32/chrome/browser/extensions/page_action_browsertest.cc
[delete] https://crrev.com/c59c9c43360f9d1467eac55bca76bb88f337979b/chrome/test/data/extensions/browsertest/title_localized/chrome-16.png
[delete] https://crrev.com/c59c9c43360f9d1467eac55bca76bb88f337979b/chrome/test/data/extensions/browsertest/title_localized/manifest.json
[delete] https://crrev.com/c59c9c43360f9d1467eac55bca76bb88f337979b/chrome/test/data/extensions/browsertest/title_localized_pa/background.js
[delete] https://crrev.com/c59c9c43360f9d1467eac55bca76bb88f337979b/chrome/test/data/extensions/browsertest/title_localized_pa/chrome-16.png
[delete] https://crrev.com/c59c9c43360f9d1467eac55bca76bb88f337979b/chrome/test/data/extensions/browsertest/title_localized_pa/manifest.json
[delete] https://crrev.com/c59c9c43360f9d1467eac55bca76bb88f337979b/chrome/test/data/extensions/browsertest/title_localized_pa/script.js
[delete] https://crrev.com/c59c9c43360f9d1467eac55bca76bb88f337979b/chrome/test/data/extensions/browsertest/title_localized_pa/simple.html

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 29

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

commit 9b0805edf7fe793249dbd0dba1cdabc232976179
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Sat Dec 29 00:39:45 2018

[Extensions] Add a common extension action API unittest

Create a common extension action API unittest that can test both page
and browser actions. This allows us to increase test coverage, and will
be useful when introducing the "action" manifest key (since we can
simply add another parameter). Move the BrowserActionUnitTest.MultiIcons
test to the shared test suite.

Bug: 893373

Change-Id: Ic560c9c5103dc6011c875b2f3b10fc844ce68756
Reviewed-on: https://chromium-review.googlesource.com/c/1384941
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619216}
[delete] https://crrev.com/5233e638fb991f2dbc161e66efa7ba87fb05fd65/chrome/browser/extensions/api/extension_action/browser_action_unittest.cc
[add] https://crrev.com/9b0805edf7fe793249dbd0dba1cdabc232976179/chrome/browser/extensions/api/extension_action/extension_action_api_unittest.cc
[modify] https://crrev.com/9b0805edf7fe793249dbd0dba1cdabc232976179/chrome/test/BUILD.gn
[delete] https://crrev.com/5233e638fb991f2dbc161e66efa7ba87fb05fd65/chrome/test/data/extensions/api_test/browser_action/multi_icons/icon19.png
[delete] https://crrev.com/5233e638fb991f2dbc161e66efa7ba87fb05fd65/chrome/test/data/extensions/api_test/browser_action/multi_icons/icon24.png
[delete] https://crrev.com/5233e638fb991f2dbc161e66efa7ba87fb05fd65/chrome/test/data/extensions/api_test/browser_action/multi_icons/icon38.png
[delete] https://crrev.com/5233e638fb991f2dbc161e66efa7ba87fb05fd65/chrome/test/data/extensions/api_test/browser_action/multi_icons/manifest.json

Sign in to add a comment