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

fix: avoid internal requests for action icons #788

Merged
merged 1 commit into from
Oct 11, 2019

Conversation

lidel
Copy link
Member

@lidel lidel commented Oct 10, 2019

This PR:

  • Checks if browser action icon really changed. We now set it only when it is really needed, removing the overhead of redraw and internal requests
  • Chrome does not support SVG in browser action, so we have to compute the path of raster version (PNG). This needs to be done only once, so the calculation is now memoized.
    • There is also a commented out version that creates rasters on the fly. For now we can't remove PNGs and switch to it: we need PNGs for use in manifest and Chrome Web Store. We may switch when CWS supports SVG at some point in the future.

Motivation

Context: every time peer count was updated, the browser action icon got set,
which caused unnecessary internal request for PNG assets every 3 seconds:

internal-requests-2019-10-10--20-47-51

This change mostly makes request logs in Network tab less noisy in Brave (#716), but is also related to addressing #721: next step will be to stop polling API status every 3 seconds.

This change checks if icon changed, and sets it only when it is needed,
removing overhead of redraw and internal requests.

Context: every time peer count was updated, the browser action icon got set,
which caused unnecessary  internal request for PNG assets on Chrome.
Chrome does not support SVG in browser action, so we also had to compute
the path of PNG (it is now memoized).
@lidel lidel merged commit 7b84a06 into master Oct 11, 2019
@lidel lidel deleted the fix/avoid-internal-requests branch October 11, 2019 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant