Skip to content
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

cherrypick: v11.15.4 reset streams on prerender only for affected browser versions #24332

Commits on May 9, 2024

  1. v11.15.4

    danjm committed May 9, 2024
    Configuration menu
    Copy the full SHA
    f9b2ea1 View commit details
    Browse the repository at this point in the history
  2. fix: reset streams on prerender only for affected browser versions (#…

    …24317)
    
    <!--
    Please submit this PR as a draft initially.
    Do not mark it as "Ready for review" until the template has been
    completely filled out, and PR status checks have passed at least once.
    -->
    
    ## **Description**
    
    The prerender regression in chromium has been resolved already.
    Resetting streams on prerendered pages no longer required for newer
    chromium versions and is actually the cause of unresponsive provider in
    many cases. This PR modifies the existing workaround to target only the
    affected chromium ranges from >=113 to <121.
    
    My bisection of chromium for the fix landed on this range of commits:
    https://chromium.googlesource.com/chromium/src/+log/55b4344edfb41dda980d197743f25a2841d498a4..c13107c16780c195bd5ec003d9198d87cdcc59dd
    
    @shanejonas and I have verified that the prerender issue has been
    resolved in the latest chrome stable. I have verified that the
    workaround is causing an issue in develop that is no longer seen in this
    branch that reverts it.
    
    [![Open in GitHub
    Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24293?quickstart=1)
    
    ## **Related issues**
    
    See: #19727
    
    ## **Manual testing steps**
    Using chrome, of course.
    Visit `chrome://settings/?search=preload` and ensure it is enabled and
    on "standard"
    
    1. On develop, make a build and load it
    2. Visit `https://voyager-snap.linea.build/`
    3. Install the snap and connect
    4. Copy `https://voyager-snap.linea.build/` to your clipboard
    5. Close all instances of `https://voyager-snap.linea.build/` that may
    be open
    6. Rapidly, open a new tab, paste the url, hit enter, see if the page is
    stuck on "Loading", if not close the tab and repeat
    7. Eventually the page should be get prerendered and stuck in a broken
    state
    
    Do the same steps above, but using the changes in this branch. The page
    should never be stuck with "Loading" if you are on a chromium version
    from 102 to 112, or 121 onwards.
    
    ## **Screenshots/Recordings**
    
    <!-- If applicable, add screenshots and/or recordings to visualize the
    before and after of your change. -->
    
    ### **Before**
    
    
    https://github.com/MetaMask/metamask-extension/assets/918701/6bda7a5d-12d2-46e4-b5d6-8562c3143977
    
    
    ### **After**
    
    
    
    https://github.com/MetaMask/metamask-extension/assets/918701/14289f0e-89eb-4534-8815-615a57e0386b
    
    
    ### Browser version targeting proof
    
    The following screenshots show the return value for
    `getIsBrowserPrerenderBroken()` on various chromium versions
    
    120:
    
    ![Screenshot 2024-04-30 at 12 47
    57 PM](https://github.com/MetaMask/metamask-extension/assets/918701/8ce52f2f-7b1d-4825-a265-9fabae1f7be4)
    
    
    119:
    
    ![Screenshot 2024-04-30 at 12 50
    57 PM](https://github.com/MetaMask/metamask-extension/assets/918701/965fe680-2579-4bd8-98db-3cf19a6dfaed)
    
    
    113:
    ![Screenshot 2024-04-30 at 11 22
    20 AM](https://github.com/MetaMask/metamask-extension/assets/918701/7b643d07-4731-444f-b888-d0a49a79feb5)
    
    112:
    ![Screenshot 2024-04-30 at 11 20
    43 AM](https://github.com/MetaMask/metamask-extension/assets/918701/80040268-c137-4d98-9dd9-661f6c78ade7)
    
    ## **Pre-merge author checklist**
    
    - [x] I’ve followed [MetaMask Coding
    Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
    - [x] I've completed the PR template to the best of my ability
    - [x] I’ve included tests if applicable
    - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
    if applicable
    - [x] I’ve applied the right labels on the PR (see [labeling
    guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
    Not required for external contributors.
    
    ## **Pre-merge reviewer checklist**
    
    - [ ] I've manually tested the PR (e.g. pull and build branch, run the
    app, test code being changed).
    - [ ] I confirm that this PR addresses all acceptance criteria described
    in the ticket it closes and includes the necessary testing evidence such
    as recordings and or screenshots.
    jiexi authored and danjm committed May 9, 2024
    Configuration menu
    Copy the full SHA
    1357fdb View commit details
    Browse the repository at this point in the history