Thanks for this change! A suggestion follows:

As written, the is_accessible method will be called once for each filediff in each diffset, per the code in review_request.py:
         for filediff in diffset.files.all():
            ic(filediff)
            for hook in FileDiffACLHook.hooks:
                try:
                    if not hook.is_accessible(diffset, filediff, user):

I think it would be more flexible, though, if the hook were called once per diffset instead.
For a Perforce example, suppose a company organizes its ACLs based on the first 3 levels in the depotFile path:
//<depot>/<project>/<branch>/...

That is, within a given depot, project, and branch, if a user has access to one file, they have access to all files. 
And suppose that a review has a fileset containing 100 filediffs:
//depot/foo/main/f00.c
//depot/foo/main/f01.c
...
//depot/foo/main/f98.c
//depot/foo/main/f99.c

In this case, the hook function would only need to test a single depot path, //depot/foo/main/..., with "p4 protects" instead of testing all 100 paths to get the result.
And in cases where there is no existing ACL pattern that can be used to optimize the tests, the hook method can still iterate over the filediffs in the diffset.

"hosting" services.

Each of these will need to be full reference paths, or Sphinx will fail to map and will probably emit warnings.

False or True?

Can we require **kwargs (or document it at least) to make future expansion possible without breaking things?

In 4.0 (maybe late 3.0.x?), we added scmtool_id, which is more stable and will be needed for registries. This should check that and compare against 'perforce'.

Two blank lines.

Can you place this alphabetically in the hook list?

As above, let's add **kwargs to any hook methods.

The pattern we use for similar hooks is to return a boolean if we have an explicit result, or None if we don't. Can we stick to that here? Make the "I don't know, someone else check" result just be None?

Shouldn't this be checking results?

Blank line between these.

Leftover debugging?

I saw the comment about letting the hook just take a diffset (I think that's maybe a worthwhile change — will discuss in that comment), so this comment depends on the outcome of that a bit, but...
We should avoid inline functions here. The function gets redefined on every call to the parent function. I think we should instead just move this out into the class.

This is a little difficult to read. How about returning the result as a variable, and then checking the variable?

The design looks sound to me. Just a few comments.

Blank line between these.

Should that be extension: "%s" or extension "%s"?
I see that the ReviewRequestApprovalHook version has the former, but I wonder if that was a typo back then.

Just to check (and we should explicitly document this everywhere): Do all hooks need to say either True or None for the result to be True, or do we want to allow any True to win?
ReviewRequestApprovalHook chains results, so that a hook can do effectively if prev_approved and .... We don't have that design for this hook (should we?), so there isn't a precedent here.
I don't honestly know what the right design is, so this is really just a question and a request to make it crystal-clear in docs/comments, specifically describing what happens with multiple hooks (important to note that hooks can be called in any order).

Can we have unit tests that test through the hook mechanism? I feel we'd want to ensure that, for N hooks (maybe 2, 3) that we get the right results for a combination of True, False, and None results.

All the logic and docs look good to me. Only a couple small things (function ordering, missing return) and I'm happy.

Private functions should go after public functions.

Private functions should go after public functions.

This function is missing an explicit return at the end. That'd result in a None, but docs say to expect True or False.

I think the only thing left I'd want to see is a change to this paragraph to hammer in the expectations of the implementation. Right now, the requirements are all in one big paragraph, and I just know people are going to mentally skip something.
Can we make the "If the user does not have ..." text onward each its own bullet point, like:

The hook is expected to return:

* ``True`` if the user does have access to all the files.
* ``False`` if the user does not have access to one or more files.
* ``None`` if the hook did not check the diff (for example, it might only check a single SCM or hosting service type, and ignore any others)

If any hook returns ``False``, the user will not have access to the diff.