Skip to content

Remove FXIOS-15286 [Telemetry] Ad cookies parsing not used#32832

Draft
janbrasna wants to merge 2 commits intomozilla-mobile:mainfrom
janbrasna:del/ads-cookies-parsing
Draft

Remove FXIOS-15286 [Telemetry] Ad cookies parsing not used#32832
janbrasna wants to merge 2 commits intomozilla-mobile:mainfrom
janbrasna:del/ads-cookies-parsing

Conversation

@janbrasna
Copy link
Copy Markdown
Contributor

📜 Tickets

Jira ticket FXIOS-15286
GitHub issue #32826

💡 Description

The values collected JS–side are not utilized further by the native helpers. This has caused WebKit crashes so it would be better to remove completely if unused.

📝 Checklist

  • I filled in the ticket numbers and a description of my work
  • I updated the PR name to follow our PR naming guidelines
  • I ensured unit tests pass and wrote tests for new code
  • If adding telemetry, I read the data stewardship requirements and will request a data review
  • If needed, I updated documentation and added comments to complex code

Copy link
Copy Markdown
Contributor Author

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

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

Before its PR even landed, it seems the only cookie parsing listed for bing eventually got removed in c144c34a within #8970 before the feature shipped. Nothing has shown since to be adding any use of this specific message data, so I'm assuming the collection on the document side is completely superfluous. @nbhasin2 Does that sound plausible from when this functionality was launched?

Comment on lines -15 to 17
'urls': getLinks(),
'cookies': getCookies()
'urls': getLinks()
};

webkit.messageHandlers.adsMessageHandler.postMessage(message);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looking at the adsMessageHandler processing it, I only ever see iterating over the links:

let body = message.body as? [String: Any],
let urls = body["urls"] as? [String] else { return }

but no cookie info (from message body["cookies"]) is ever parsed.

It also appears the whole message payload is never stored raw, so that would hint at only parsing the explicit values, which is currently just the links.

@mobiletest-ci-bot
Copy link
Copy Markdown

🧹 Tidy commit

Just 2 file(s) touched. Thanks for keeping it clean and review-friendly!

✅ New file code coverage

No new file detected so code coverage gate wasn't ran.

Generated by 🚫 Danger Swift against 7669339

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.

2 participants