Fix WebLocksMixin state initialization.

[rhashimoto]

WebLocksMixin was calling the jLock() method from jFileControl() if its state needed initialization, which was sloppy and enabled a bug if any subclasses implemented their own jLock(), like OPFSAdaptiveVFS. This PR moves initialization into a dedicated private method.

[gemini-code-assist]

Summary of Changes

Hello @rhashimoto, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the state initialization mechanism within the WebLocksMixin class. By extracting the logic for creating and retrieving file lock states into a dedicated private method, it streamlines the codebase, prevents potential bugs arising from method overrides in subclasses, and ensures a consistent approach to managing lock state across various operations.

Highlights

• Centralized State Initialization: The logic for initializing the WebLocksMixin state has been extracted and moved into a new dedicated private helper method, #getLockState.
• Improved Modularity and Consistency: Redundant state initialization code has been removed from jLock() and jFileControl(), with all relevant methods now consistently using #getLockState.
• Bug Prevention: This change addresses a potential bug where subclasses overriding jLock() could lead to incorrect behavior when jFileControl() implicitly called it for state creation.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the lock state initialization in WebLocksMixin by introducing a new private method #getLockState. This is a good change that fixes a bug in jFileControl and improves the overall design. My review includes a couple of suggestions. One is a high-severity issue in jUnlock where the new method introduces an unintended side effect; I've recommended reverting that specific part of the change. The other is a medium-severity suggestion to improve the readability of the new #getLockState method. With these changes, the PR will be in great shape.

[gemini-code-assist]

Using #getLockState(fileId) here introduces a change in behavior for jUnlock. The new implementation creates a lock state if one doesn't exist. This is unnecessary for an xUnlock operation on a file that was never locked, and it adds a new state to the #mapIdToState map. The previous implementation handled this case correctly by doing nothing.

It would be better to revert to getting the state directly from the map to avoid this side effect. With this change, the optional chaining on the next line (lockState?.type) will be correct and necessary again.

Suggested change

	const lockState = this.#getLockState(fileId);
	const lockState = this.#mapIdToState.get(fileId);

[gemini-code-assist]

The implementation of #getLockState can be slightly simplified to improve readability by separating the state creation from its retrieval. This avoids reassigning the lockState variable.

  #getLockState(fileId) {
    if (!this.#mapIdToState.has(fileId)) {
      // The state doesn't exist yet so create it.
      const name = this.getFilename(fileId);
      this.#mapIdToState.set(fileId, {
        baseName: name,
        type: VFS.SQLITE_LOCK_NONE,
        writeHint: false
      });
    }
    return this.#mapIdToState.get(fileId);
  }