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

Replace setInterval with chrome alarms for MetaMetrics FinalizeEventFragment #16003

Merged
merged 4 commits into from
Oct 4, 2022

Conversation

NiranjanaBinoy
Copy link
Contributor

@NiranjanaBinoy NiranjanaBinoy commented Sep 27, 2022

Explanation

We are replacing setInterval with chrome alarm to schedule the call to FinalizeEventFragment() every 1 min. The current interval window for FinalizeEventFragment() is 30 sec; we are updating that to 1 min since chrome alarms require the interval to be at least 1 mins and replace setInterval with chrome alarms.

More Information

Fixes #15933

Screenshots/Screencaps

Before

After

Manual Testing Steps

  1. Load the application, and in the service worker, try using breakpoints to check if the alarm is getting created.
  2. Breakpoint or console log inside chrome.alarm.onalarm.addListener() and inside getall() to verify the alarm is getting triggered at 1 mins interval.

Pre-Merge Checklist

  • PR template is filled out
  • IF this PR fixes a bug, a test that would have caught the bug has been added
  • PR is linked to the appropriate GitHub issue
  • PR has been added to the appropriate release Milestone

+ If there are functional changes:

  • Manual testing complete & passed
  • "Extension QA Board" label has been applied

@NiranjanaBinoy NiranjanaBinoy requested a review from a team as a code owner September 27, 2022 22:08
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@NiranjanaBinoy NiranjanaBinoy self-assigned this Sep 27, 2022
@NiranjanaBinoy NiranjanaBinoy added the PR - P1 identifies PRs deemed priority for Extension team label Sep 27, 2022
brad-decker
brad-decker previously approved these changes Sep 28, 2022
);

if (hasAlarm) {
Object.values(this.store.getState().fragments).forEach(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this Object.values.forEach block a helper function? It looks duplicated here and within the setTimeout

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NiranjanaBinoy we could make a 'finalizeAbandonedFragments' method that does this.

@metamaskbot
Copy link
Collaborator

Builds ready [1aefe89]
Page Load Metrics (2303 ± 79 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint92143111147
domContentLoaded18682536227917483
load19802556230316579
domInteractive18682536227917483

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [c8b3f43]
Page Load Metrics (2226 ± 65 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint90163105168
domContentLoaded19832479221412259
load19832535222613565
domInteractive19832479221412259

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [0e7be61]
Page Load Metrics (2415 ± 69 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint972485235516248
domContentLoaded21322739238613766
load21322809241514469
domInteractive21322739238613766

highlights:

storybook

@NiranjanaBinoy NiranjanaBinoy merged commit 29c2b13 into develop Oct 4, 2022
@NiranjanaBinoy NiranjanaBinoy deleted the alarm-setInterval-metametrics branch October 4, 2022 17:03
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR - P1 identifies PRs deemed priority for Extension team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace setInterval for MetaMetrics FinalizeEventFragment
4 participants