Should be throw new FormatError(...), since that's used for errors with the PDF data itself.

Please make this method async, since the data fetching below (i.e. the Dict.get(...) calls) could throw which would thus break the OperatorList parsing, and that way you'll be able to (easier) deal with any errors gracefully (relying on the existing ignoreErrors functionality).

This line looks superfluous, since fn === OPS.beginMarkedContentProps must hold in order for this block to have been entered!?

Similar to all other methods here, see e.g. get permissions() above, please wrap this in try...catch to prevent breaking errors if data fetching fails (since Dict.get(...) calls could throw).

Will get optionalContentConfig() always return the same data for a document with Optional Content groups?
If so, please shadow the return values to avoid having to repeatedly re-parsing this for every single page. (Or at least, please shadow the return null; cases.)

Left-over debugging statement?

With parseMarkedContentProps being asynchronous, as suggested above, this would need to become something like this (which thus handles any errors gracefully):

next(
  self.parseMarkedContentProps(args[1], resources)
    .then(data => {
       operatorList.addOp(OPS.beginMarkedContentProps, ["OC", data]);
    })
    .catch(reason => {
      if (reason instanceof AbortException) {
        return;
      }
      if (self.options.ignoreErrors) {
        self.handler.send("UnsupportedFeature", {
          featureId: /* Some new UNSUPPORTED_FEATURES id */,
        });
        warn(`getOperatorList - ignoring beginMarkedContentProps: "${reason}".`);
        return;
      }
      throw reason;
    })
);
return;

Nit: With parseMarkedContentProps being asynchronous, as suggested below, this would need to be groupOptions.optionalContent = await this.parseMarkedContentProps( instead.

It would in my opinion be, a little bit, clearer if you simply enclosed this entire code-block

Also, unless I'm mistaken there's no similar checks in the showType3Text below. Don't you need to account for this.contentVisible there as well?

Which test-case required moving this code, and is there another way to deal with that?
(Since the commit message says: - Reject the InternalRenderTask earlier now that InitializeGraphics is waits for another promise.)

I've not actually tested it, but based on reading the code I'm not entirely convinced that moving this code will actually do the right thing in general.
Currently initializeGraphics will throw

With your changes here, it doesn't seem like you're actually guaranteed that complete is ever called correctly!?

It was the test that calls render twice. It doesn't seem to be failing anymore. This code is quite the mix of callbacks and old style promises.

I missed this previously: You want this.xref.fetchIfRef(groupRef); instead here, since this.xref is guaranteed to be defined whereas a Dict instance may not always have an xref property.

Missing e.g. the word "read" here?

warn(`Unable to read optional content config: ${ex}`);

Since this data is being sent between threads, whatever data this contains much be compatible with the "structured clone algorithm". Hence you may want to add at least some basic validation of these two properties since otherwise a corrupt PDF document might refuse to render altogether.

Since this data is being sent between threads, whatever data this contains much be compatible with the "structured clone algorithm". Hence you may want to add at least some basic validation of the P property since otherwise a corrupt PDF document might refuse to render altogether.

According to the specification, https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/PDF32000_2008.pdf#G7.3883825, this should be a Name. In the OptionalContentConfig class you're accessing them as if they were mere strings, which I don't think will work!?

I believe that you'll need to either validate that it's a name and (essentially) return optionalContent.get("P").name, here, or possibly return the name as-is a and fix the lookup in OptionalContentConfig instead.

I thought there was a test PDF for this, but appears there aren't any with a policy attribute. If I have some time I'll try to make some more tests.

Since this data is being sent between threads, whatever data this contains much be compatible with the "structured clone algorithm". Hence you may want to add at least some basic validation of the Name/Creator/BaseState properties since otherwise a corrupt PDF document might refuse to render altogether.

Just wondering: Any particular reason to not simply pass the results when creating the OptionalContentConfig instance, and thus essentially inline the initialize method in the constructor?

I had plans of doing some unit testing this differently. Not going to do that now, so I'll merge it.

Since this is, I believe, a string; don't you need to wrap it in stringToPDFString() to avoid issues?

Edit: Issue #7283 is one example which requires this.

Since these are, I believe, strings; don't you need to wrap them in stringToPDFString() to avoid issues?

Edit: Issue #7283 is one example which requires this.

According to the specification, https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/PDF32000_2008.pdf#G7.3883884, this should be a Name. In the OptionalContentConfig class you're accessing them as if they were mere strings, which I don't think will work!?

I believe that you'll need to either validate that it's a name and (essentially) return config.get("BaseState").name, here, or possibly return the name as-is a and fix the lookup in OptionalContentConfig instead.

According to the specification, https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/PDF32000_2008.pdf#G7.3883825, this should be a Name. In the OptionalContentConfig class you're accessing them as if they were mere strings, which I don't think will work!?

I believe that you'll need to either validate that it's a name and (essentially) return optionalContent.get("P").name, here, or possibly return the name as-is a and fix the lookup in OptionalContentConfig instead.

Given that every call-sites seem to pass in this.markedContentStack manually, any reason not to make this a "normal" method and simply handle the stack internally instead?
E.g. something like this

_computeContentVisible() {
  const state = this.markedContentStack;
  for (let i = stack.length - 1; i >= 0; i--) {
    if (!stack[i].visible) {
      return false;
    }
  }
  return true;
},

and the call-sites could then simply read this.contentVisible = this._computeContentVisible();

Would it actually make sense to simply move this check into the next handling below and throw a FormatError instead?
I.e. something similar to the next code that's used e.g. when parsing OPS.paintXObject earlier in this file.

Really sorry about the back-and-forth in some of these comments, but it's quite a lot of added code/functionality to "unpack" here and at first glance I unfortunately overlooked some of the finer details.
The overall functionality seem to work very nicely though :-)

I started to do do this, but sometimes the marked content isn't an "OC" which then causes us to wait async when we really don't need to.

Nit: Would it make sense to move this to occur earlier in the method, since the groupsData parsing above seems pointless in the !defaultConfig case?

Nit: expressions

This handler can be shortened a bit by using an anonymous function like the one below.

Shouldn't @see be on a new line for JSDoc?

Nit: automatically be fetched

Nit: 2020

All form XObjects can have an OC entry, not just transparency groups that have a Group entry. Then you may need to pass the parsed OC value also to paintFormXObjectBegin/End and handle it in canvas.js.

Good catch, I'll just insert a beginMarkedContentProps before the form object so I don't have to handle this for both.