Closed
Bug 1047820
Opened 10 years ago
Closed 6 years ago
Remove getItemAnnotationNames
Categories
(Toolkit :: Places, defect, P3)
Toolkit
Places
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: mak, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: perf)
We should check the consumers of getAnnotationNames and figure what they try to do, it's possible they are doing something very unefficient like getting all names and then looping through them
Reporter | ||
Updated•10 years ago
|
Blocks: PlacesJank
Reporter | ||
Updated•9 years ago
|
Priority: -- → P3
Reporter | ||
Updated•8 years ago
|
Summary: Investigate usage of getAnnotationNames → Investigate usage of getPage/ItemAnnotationNames
Reporter | ||
Updated•7 years ago
|
Priority: P3 → P2
Updated•7 years ago
|
Assignee: nobody → standard8
Comment 1•7 years ago
|
||
After bug 1047819 lands, we'll be in this state:
- getItemAnnotationNames
Used by PlacesUtils.getAnnotationsForItem and PlacesController._buildSelectionMetadata.
- getPageAnnotationNames
Used by PlacesController._buildSelectionMetadata
The _buildSelectionMetaData cases are getting annotation details for the items being selected. This is then used when building the context menu to decide what is/isn't displayed based on the 'selection' attribute:
https://searchfox.org/mozilla-central/search?q=selection&case=true®exp=false&path=placesContextMenu.inc.xul
The only annotation that's currently listed in the selection attribute is "livemark/feedURI" - we can obtain that via hasCachedLivemarkInfo (the code already does for finding out if there's a livemark parent), so then we don't actually need to get any of the annotations.
For the remaining getItemAnnotationNames case, I think this could be combined with a new version of getItemAnnotationInfo - which would save going twice across the xpcom boundaries, and two separate SQL lookups.
However, what I'm not quite sure about with that is the idl interface, as can we return an array as a variant, or are we going to need something like mozIAnnotatedResult?
If it is another interface, then I'm not sure it is worth combining these functions at the moment, given the uncertain futures of annotations etc.
Reporter | ||
Comment 2•7 years ago
|
||
(In reply to Mark Banner (:standard8) (afk until 3rd April) from comment #1)
> The only annotation that's currently listed in the selection attribute is
> "livemark/feedURI" - we can obtain that via hasCachedLivemarkInfo (the code
> already does for finding out if there's a livemark parent), so then we don't
> actually need to get any of the annotations.
What about getPageAnnotationNames? I don't see any rule that uses this info, can it be removed already?
The only page anno that come to my mind are:
downloads/destinationFileURI
downloads/metaData"
URIProperties/characterSet
None of these seem to be involved with _buildSelectionMetadata
Removing usage from _buildSelectionMetadata seems to go a long way in the right direction.
> However, what I'm not quite sure about with that is the idl interface, as
> can we return an array as a variant, or are we going to need something like
> mozIAnnotatedResult?
>
> If it is another interface, then I'm not sure it is worth combining these
> functions at the moment, given the uncertain futures of annotations etc.
Likely yes, we should wait for this.
Reporter | ||
Comment 3•7 years ago
|
||
I filed bug 1450223 for _buildSelectionMetadata, the remaining work here can wait
Priority: P2 → P3
Updated•7 years ago
|
Assignee: standard8 → nobody
Reporter | ||
Comment 4•7 years ago
|
||
morphing. This is not trivial because it's used by D&D & clipboard code (through wrapNode and serializeNode), but long term we may discover we don't really need item annotations... and then this would not be a problem.
Summary: Investigate usage of getPage/ItemAnnotationNames → Remove getItemAnnotationNames
Updated•7 years ago
|
Reporter | ||
Comment 5•6 years ago
|
||
the plan is to remove item annotations.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•