[Extensions] Remove handling for .user.js files [chromium/src : main]
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 6 comments![]( "Open in Gerrit")
Hi Devlin,
This Cl removes broken `extensions::ConvertUserScriptToExtension()` method and its callers (including tests).
This CL removes special handling for files ending in `.user.js` when they are dropped into `chrome://extensions` page. This functionality relied on Manifest Version 1 support because `extensions::ConvertUserScriptToExtension()` does not explicitly insert the `"manifest_version"` key, so `extensions::Manifest:: GetManifestVersion()` defaults manifest to `1`[1]. This drop functionality is already broken since migration to Manifest Version 3 breaks earlier versions.
This patch not affect regular extension load. In particular, it does not affect extensions with MV3 and `"converted_from_user_script"` manifest key. This patch does not touch `extensions::Extension` class in any way. All `.user.js` files which were pulled into earlier versions of Chrome and then were packed into an archive and uploaded to Chrome Web Store, and then updated to be MV3-compliant would still be supported since they already underwent the conversion.
Alternatively, we can fix extension loading by fixing `extensions::ConvertUserScriptToExtension()` by adding `root.Set(manifest_keys::kManifestVersion, 3);`. This one liner would fix converted extension load, but a lot of bugs specific to converted User Scripts would remain.
[1] https://source.chromium.org/chromium/chromium/src/+/main:extensions/common/manifest.cc;l=107
if (extensions::UserScript::IsURLUserScript(url, mime_type)) {We should not force-download User Scripts. After removing this clause, User Scripts are treated as regular files.
installer->InstallCrx(item->GetFullPath());Since we removed `extensions::UserScript::IsURLUserScript()`, then this code is reachable only by regular (MV3) extensions.
return RespondNow(Error(kUserScriptsNotSupportedError));Here in `DeveloperPrivateInstallDroppedFileFunction` we can just abort extension install. This error gets logged into the console. If needed, I can add a user-visible warning.
if (download_item.GetMimeType() == Extension::kMimeType ||In `IsExtensionDownload()` I remove call to `UserScript::IsURLUserScript()` so it User Scripts will be handled like regular files.
bool UserScript::IsURLUserScript(const GURL& url,We can remove every call to `UserScript::IsURLUserScript()` and at very call site pretend that it always returns `false`.
Related details
- Devlin Cronin
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Devlin Cronin (Gerrit)
Devlin Cronin added 4 comments![]( "Open in Gerrit")
EXPECT_FALSE(api_test_utils::RunFunction(function.get(), "[]", profile()))
<< "User script support was removed.";
EXPECT_EQ("User Scripts are not supported.", function->GetError());nit: prefer RunFunctionAndReturnError()
if (MatchesExtension(file, FILE_PATH_LITERAL(".user.js"))) {this method is only used by the chrome://extensions page, so we should update that to not call into here with .user.js files (updating [this](https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/resources/extensions/drag_and_drop_handler.ts;l=71;drc=409a2f3667e632ee01450646335641b80e5ceab3) code)
void ConvertUserScriptOnSharedFileThread();remove this declaration
Related details
- Anton Bershanskyi
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 4 comments![]( "Open in Gerrit")
EXPECT_FALSE(api_test_utils::RunFunction(function.get(), "[]", profile()))
<< "User script support was removed.";
EXPECT_EQ("User Scripts are not supported.", function->GetError());Anton Bershanskyinit: prefer RunFunctionAndReturnError()
I removed test `DeveloperPrivateApiUnitTest.InstallDroppedFileUserScript` because I removed special handling of `.user.js` files from TS and `DeveloperPrivateInstallDroppedFileFunction` does not handle `.user.js` files any more.
if (MatchesExtension(file, FILE_PATH_LITERAL(".user.js"))) {this method is only used by the chrome://extensions page, so we should update that to not call into here with .user.js files (updating [this](https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/resources/extensions/drag_and_drop_handler.ts;l=71;drc=409a2f3667e632ee01450646335641b80e5ceab3) code)
Thanks, I updated the TS file, as you suggested.
void ConvertUserScriptOnSharedFileThread();Anton Bershanskyiremove this declaration
Done
Related details
- Devlin Cronin
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |