New issue
Advanced search Search tips

Issue 835392 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-04-23
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Update mobile_app CrOS component extension to manifest v2

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

Issue description

Manifest v1 extensions will no longer be loaded in M67 and onwards, but the mobile_app extension is manifest v1.  We need to update it to v2 (which, luckily, appears to be trivial).

Longer-term, it would be nice to remove.  See issue 835391.
 
Labels: -M67 M-67
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 20 2018

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

commit 8f9c7d85f27e4d5d7fb4da20d13ab6b66cd4269e
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Apr 20 23:25:08 2018

[CrOS Extensions] Update Mobile Activation extension to manifest v2

Manifest v1 extensions will no longer be loaded in M67. We need to
update this extension to manifest v2.

Update the manifest, and add a trivial browser test to at ensure the
extension is properly loaded on CrOS (since it looks like we don't have
any other coverage for this).

Bug:  835392 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I36734dd22b85a93639cf6603841ada430b3bdf50
Reviewed-on: https://chromium-review.googlesource.com/1022258
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552517}
[add] https://crrev.com/8f9c7d85f27e4d5d7fb4da20d13ab6b66cd4269e/chrome/browser/extensions/chromeos_component_extensions_browsertest.cc
[modify] https://crrev.com/8f9c7d85f27e4d5d7fb4da20d13ab6b66cd4269e/chrome/browser/resources/chromeos/mobile_app/manifest.json
[modify] https://crrev.com/8f9c7d85f27e4d5d7fb4da20d13ab6b66cd4269e/chrome/test/BUILD.gn

NextAction: 2018-04-23
Status: Fixed (was: Started)
This should be fixed, and will likely need a merge.  Setting a reminder for Monday to request a merge if all looks good.
The NextAction date has arrived: 2018-04-23
Labels: Merge-Request-67
Has this been tested against some number of boards?  Want to verify prior to merge.
Project Member

Comment 7 by sheriffbot@chromium.org, Apr 24 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
I have not tested this against any specific CrOS devices beyond what are running on our trybots and waterfalls, but it should be a reasonably low-risk change.

I'll defer to stevenjb@ for CrOS expertise on whether this should be sufficient, or if there's more we should test.
I suspect the extension is actually unused, but we have not been able to prioritize verifying that. In the meanwhile this change is straightforward and low risk and we should merge it in case the extension API is in fact used (my understanding is that it won't work at all without this change).

Per #9, I'll merge.
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 24 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bf1a089e1e1d38674c25f7a7cadb56e3a973a31

commit 5bf1a089e1e1d38674c25f7a7cadb56e3a973a31
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Apr 24 18:51:03 2018

[Merge 67][CrOS Extensions] Update Mobile Activation extension to manifest v2

Manifest v1 extensions will no longer be loaded in M67. We need to
update this extension to manifest v2.

Update the manifest, and add a trivial browser test to at ensure the
extension is properly loaded on CrOS (since it looks like we don't have
any other coverage for this).

TBR=rdevlin.cronin@chromium.org

(cherry picked from commit 8f9c7d85f27e4d5d7fb4da20d13ab6b66cd4269e)

Bug:  835392 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I36734dd22b85a93639cf6603841ada430b3bdf50
Reviewed-on: https://chromium-review.googlesource.com/1022258
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552517}
Reviewed-on: https://chromium-review.googlesource.com/1026373
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#260}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[add] https://crrev.com/5bf1a089e1e1d38674c25f7a7cadb56e3a973a31/chrome/browser/extensions/chromeos_component_extensions_browsertest.cc
[modify] https://crrev.com/5bf1a089e1e1d38674c25f7a7cadb56e3a973a31/chrome/browser/resources/chromeos/mobile_app/manifest.json
[modify] https://crrev.com/5bf1a089e1e1d38674c25f7a7cadb56e3a973a31/chrome/test/BUILD.gn

Sign in to add a comment