-
Notifications
You must be signed in to change notification settings - Fork 1
Eliminate Broadcast Channel bfcache eviction method #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… cache is disabled in DevTools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @westonruter, I'm Gemini Code Assist1! 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 streamlines the bfcache invalidation strategy by completely removing the Broadcast Channel-based eviction method. This change addresses identified issues where the Broadcast Channel method was only partially effective and could lead to unexpected behavior with the HTTP cache, making the pageshow event-based invalidation the sole and more robust mechanism for ensuring user privacy after logout.
Highlights
- Removed Broadcast Channel Invalidation: The entire Broadcast Channel mechanism for evicting pages from the bfcache has been eliminated. This includes the removal of its emitter and listener JavaScript modules, associated PHP constants, functions, and script enqueues.
- Consolidated Invalidation Logic: The
pageshowevent-based bfcache invalidation, which relies on a session token cookie, is now the singular and primary method for protecting user privacy after logout, as it proved more robust. - Documentation Update: The
README.mdhas been updated to accurately reflect that only one safeguard (the cookie-based method) is in place for bfcache invalidation, removing outdated references to the Broadcast Channel method. - Code Cleanup and Refactoring: PHP constants for script module IDs have been moved from a dedicated interface file (
includes/interface-script-module-ids.php, which was removed) intoincludes/script-loader.php, simplifying the project structure and centralizing constant definitions.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 or fill out our survey to provide feedback.
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
-
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. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the bfcache invalidation mechanism by removing the BroadcastChannel approach, which was found to be unreliable. It now solely relies on a cookie-based session token and a pageshow event handler. The changes are logical and well-explained in the PR description. My review has identified two critical issues that need to be addressed: a fatal PHP error due to a missing file reference update, and a JavaScript runtime error caused by an inconsistent module ID in a renamed file. I've also included a suggestion to improve the clarity of the README.md.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
It turns out that using Broadcast Channel to evict pages from bfcache is only partially successful. I had been testing with DevTools open in Chrome and Firefox, where “Disable Cache” is checked. In my testing, as soon as a message is broadcast, attempting to go back to a page which was in bfcache would send me to the login screen. However, this was because the page was evicted from bfcache and the browser's HTTP cache was disabled. If, however, I tried testing with DevTools closed and the HTTP cache is not disabled, then the broadcast channel invalidation method was causing a problem: it was indeed evicting the page from bfcache, but the page without
no-storeremained in HTTP cache. Thepageshowevent would report to me the bfcache blocking reason as beingbroadcastchannel-message. This meant that a page was restored from HTTP cache in the same way as a page is restored when re-opening a closed tab. So using Broadcast Channel was worse because thepageshowevent'spersistedproperty wasfalse, and thus it was not able to forcibly evict itself.