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

core(installable-manifest): pipeline-restarted check #13365

Merged
merged 8 commits into from
Nov 17, 2021

Conversation

adamread
Copy link
Contributor

Summary
This adds the pipeline-restarted installable logging message to the installable-manifest audit.

This change brings the installable-manifest audit in line with the messages included in Chromium, allowing the automated comparison to run without complaining that something is missing.

The related test is also no longer skipped.

The pipeline-restarted message was introduced in Chromium commit 03777af. The message indicates that the app has been uninstalled, possibly in a context outside the currently active tab, and that the installability checks are being reset. This is only used on desktop. As Lighthouse won't be uninstalling the app, we filter the message out.

Related Issues/PRs

This is the first half of Issue 13147

When a desktop PWA is uninstalled but has other open windows in scope,
the pipeline-restarted message will be logged, signalling that the app
install banner state has been reset.

pipeline-restarted will be ignored by Lighthouse, as there is currently
no reason audit the behavior of desktop PWAs after the app has been
uninstalled.

The related test has also been updated to run again.
Minor spelling mistake.
Fixing formatting errors
Re-adding protocol-timeout to my branch.
@adamread adamread requested a review from a team as a code owner November 12, 2021 20:13
@adamread adamread requested review from connorjclark and removed request for a team November 12, 2021 20:13
@google-cla google-cla bot added the cla: yes label Nov 12, 2021
@adamread adamread changed the title Issue 13147 pipline restarted core(Issue 13147) - pipline restarted installability check Nov 17, 2021
@adamread adamread changed the title core(Issue 13147) - pipline restarted installability check core:(Issue 13147) - pipline restarted installability check Nov 17, 2021
@adamread adamread changed the title core:(Issue 13147) - pipline restarted installability check core(Issue 13147): - pipline restarted installability check Nov 17, 2021
@adamread adamread changed the title core(Issue 13147): - pipline restarted installability check core(Issue 13147): pipline restarted installability check Nov 17, 2021
@adamread adamread changed the title core(Issue 13147): pipline restarted installability check core(issue 13147): pipline restarted installability check Nov 17, 2021
Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

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

Very nice!

Little odd to translate this string we'll never show in our report, but there is still the JSON programatic users to consider, so I suppose it's necessary.

@connorjclark connorjclark changed the title core(issue 13147): pipline restarted installability check core(installable-manifest): pipeline-restarted check Nov 17, 2021
@connorjclark connorjclark merged commit 7eedb31 into master Nov 17, 2021
@connorjclark connorjclark deleted the issue-13147-pipline-restarted branch November 17, 2021 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants