Closed Bug 1743749 Opened 2 years ago Closed 2 months ago

Clean up use of atoms for cache keys


(Core :: Disability Access APIs, task)




118 Branch
Tracking Status
firefox118 --- fixed


(Reporter: Jamie, Assigned: Jamie)


(Blocks 1 open bug)


(Whiteboard: [ctw-postship])


(2 files)

We use AccAttributes for the CTW cache and AccAttributes is keyed by nsAtom. This has some advantages as compared to an enum: we don't need separate data structures for object/text attributes and the CTW cache and we get readable logging for the keys for free. However, a major disadvantage is that because we try to avoid polluting the nsGkAtoms namespace, we end up with some unclear/unintuitive keys; e.g. nsGkAtoms::line for text line start boundaries, nsGkAtoms::style for text attributes, etc.

We need to find a way to make this more obvious and readable. We can either alias the atoms in our own namespace or switch to using an enum after all.

I looked into various approaches of aliasing. We can't use an enum because enums have to be convertable to int. We can't use a using declaration because they only work for types, not values. What does work is this:

class CacheKey {
  static constexpr nsStaticAtom* TextLineStarts = nsGkAtoms::line;
Whiteboard: [ctw-postship]
Assignee: nobody → jteh

This was done with the following Python script:

import re

cacheConsts = open("accessible/base/CacheConstants.h", "rt").read()
aliases = {
  alias: atom
  for atom, alias in
    r'static constexpr nsStaticAtom\*\s+(.*?)\s+=\s+(nsGkAtoms::.*?);',

RE_ATOM = re.compile(r'(fields->SetAttribute|(?:mCachedFields|aFields)->(?:GetAttribute|GetAttributeRefPtr|GetMutableAttribute|HasAttribute|Remove|SetAttribute)(?:<.+>)?)(\(\s*)(nsGkAtoms::[a-zA-Z_]+)')

def repl(m):
  # Group 3 is the atom.
  alias = aliases.get(
  if not alias:
    # No alias for this atom. Return input unaltered.
  alias = "CacheKey::" + alias
  # Groups 1 and 2 should be returned unaltered. Group 3 (the atom) is replaced
  # with the alias.
  return + + alias

for fn in (
  # Found with: git grep -l 'ields->'
  input = open(fn, "rt").read()
  output = RE_ATOM.sub(repl, input)
  open(fn, "wt").write(output)

The following patches are waiting for review from an inactive reviewer:

ID Title Author Reviewer Status
D184667 Bug 1743749 part 1: Create aliases for RemoteAccessible cache keys. Jamie morgan: Back Aug 7, 2023
D184791 Bug 1743749 part 2: Replace usage of atoms as cache keys with the new CacheKey aliases. Jamie morgan: Back Aug 7, 2023

:Jamie, could you please find another reviewer?

For more information, please visit BugBot documentation.

Flags: needinfo?(jteh)
Flags: needinfo?(jteh)
Pushed by
part 1: Create aliases for RemoteAccessible cache keys. r=nlapre
part 2: Replace usage of atoms as cache keys with the new CacheKey aliases. r=nlapre
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch
You need to log in before you can comment on or make changes to this bug.