[Extensions] Extract NameInfo from Extension [chromium/src : main]
Anton Bershanskyi (Gerrit)
Anton Bershanskyi added 1 comment![]( "Open in Gerrit")
Hi Devlin,
this moves parsing of `"name"` and `"short_name"` to a new `NameHandler`. Resulting CL is a bit long, but splitting the move into multiple steps does not reduce the diff that much. If you like, I can do it in two steps like `ParseAppURLs()`. Also, I believe existing tests already cover the call function order behavior sufficiently.
At a later date, I plan to add few tests for differences in `LoadName()` and `LoadShortName()` and hopefully even further down the line harmonize them.
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 1 comment![]( "Open in Gerrit")
Thanks, Anton!
I'm not sure we want to go quite this far. The motivation and purpose of ManifestHandlers is that Extension had become a monstrosity with way too much knowledge of different unrelated subsystems, including complexities from different item types (app, hosted app, etc), platform-specific, APIs, etc. We needed to move those out.
But these required / core features -- name, manifest_version, and version, arguably along with some of the other extremely-basic, API-agnostic fields like short_name, etc -- are fairly core to the item and don't introduce the same complexities the others did. I think it's safe to actually leave these here on the extension, both for the parsing and the retrieval.
WDYT?
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 1 comment![]( "Open in Gerrit")
Thanks, Anton!
I'm not sure we want to go quite this far. The motivation and purpose of ManifestHandlers is that Extension had become a monstrosity with way too much knowledge of different unrelated subsystems, including complexities from different item types (app, hosted app, etc), platform-specific, APIs, etc. We needed to move those out.
But these required / core features -- name, manifest_version, and version, arguably along with some of the other extremely-basic, API-agnostic fields like short_name, etc -- are fairly core to the item and don't introduce the same complexities the others did. I think it's safe to actually leave these here on the extension, both for the parsing and the retrieval.
WDYT?
This makes sense. `"name"` and `"short_name"` were the last attributes of extension manifest I wanted to move out of `Extension` class.
If we are not moving `"name"` and `"short_name"`, then I believe now the enumeration of `ManifestHandler`-s is stable and we can simplify `ManifestRegistry` to avoid sorting `ManifestHandlers` on every process start.
Related details
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. |