Etienne Segonzac (:etienne) Reporter
	Description • 9 years ago

Similiraly to what places.js is doing here [1]

[1] https://github.com/mozilla-b2g/gaia/blob/f3ccc4c0edb0e0ebc9ea0bac3bb0ef4799554651/apps/system/js/places.js#L121-123

	Marshall Culpepper [:marshall_law] Assignee
	Comment 1 • 9 years ago

Hrm.. is it possible to load a private browser directly from the launcher?

	Gregor Wagner [:gwagner]
	Comment 2 • 9 years ago

Francis, what are your thoughts on this?

	Candice Serran (:cserran)
	Comment 3 • 9 years ago

Metrics is a committed 2.2 feature. Needs verification on how this works on desktop

	Kevin Grandon :kgrandon
	Comment 4 • 9 years ago

Etienne, Marshall - Do we know what desktop/fennec does here? Are we sending data that users don't want to be sent, or should be anonymized?

	Etienne Segonzac (:etienne) Reporter
	Comment 5 • 9 years ago

(In reply to Kevin Grandon :kgrandon from comment #4)
> Etienne, Marshall - Do we know what desktop/fennec does here? Are we sending
> data that users don't want to be sent, or should be anonymized?

TBH I opened this bug because I would feel safer with a clear check in app_usage_metrics.js.
Haven't actually checked that anything is leaking.

I just don't want to have to audit every window manager change to make sure we're not dispatching an event that shouldn't be part of metrics.

	Marshall Culpepper [:marshall_law] Assignee
	Comment 6 • 9 years ago

I don't think Desktop does any kind of application usage tracking.

I've taken a look at the code that Etienne linked again, and AFAICT places.js is only handling events that AppUsageMetrics doesn't, specifically:
* applocationchange
* apptitlechange
* appiconchange
* apploaded

AppUsageMetrics only pays attention to these events in the application lifecycle (outside of lock screen / homescreen events):
* appopened
* activitycreated
* attentionopened
* attentionclosed

I guess I'm mainly unclear whether/when an app is actually flagged as a private browser. We aren't tracking URL changes in apps, just the entry points when apps are launched, and any activity or attention windows that are opened from there. Is it plausible that an activity or attention window could be opened from a private browser?

	Kevin Grandon :kgrandon
	Comment 7 • 9 years ago

Ok, so it sounds like we are all clear from the app launch metrics. I think we can probably close this then as long as bug 1126524 did not regress this, and start sending navigation info when in a private browser. Just need to verify that first, I'll take a look.

	Kevin Grandon :kgrandon
	Comment 8 • 9 years ago

It looks like we might indeed record search info when in private browsers: https://github.com/mozilla-b2g/gaia/commit/0cd14e8b64d5e750588adacf4ad027aa90889648#diff-cc8fa85f73ada24b3dea2bce82e86e92R563

I guess the question is if this data is anonymized enough (I think it might be), that we are ok collecting it with private browsing searches? It also doesn't persist on the device in any viewable form, so I think we should be ok. We should hash this out with UX/policy people I suppose.

	Marshall Culpepper [:marshall_law] Assignee
	Comment 9 • 9 years ago

For the search metrics we are simply recording the number of searches done on a specific provider, we don't actually collect search terms themselves.

	Marshall Culpepper [:marshall_law] Assignee
	Comment 10 • 9 years ago

OK a quick update, I have a sample batch of app usage metrics that was collected when I tried opening a private browser window and searching within the private window here:
https://pastebin.mozilla.org/8825165

tl;dr We are counting searches performed in private windows (but again, not the search terms) and app usage looks to just be filed under search.gaiamobile.org.

Ideally I think under a private window, we shouldn't be counting searches. App usage in a private window looks the same as a regular window, seemingly pretty innocuous. Thoughts?

	Marshall Culpepper [:marshall_law] Assignee
	Comment 11 • 9 years ago

We just had a quick discussion about this, and concluded that we should not record app usage info or searches for private browsing windows. Looks like this already nomed for 2.2, so I'll try to get this fixed ASAP.

	Shing Lyu [:shinglyu]
	Comment 12 • 9 years ago

FC is coming (4/6), we need to be quick to land this.

	Shing Lyu [:shinglyu]
	Comment 13 • 9 years ago

Hi Marshall, could you provide some status update? Thanks.

	Marshall Culpepper [:marshall_law] Assignee
	Comment 14 • 9 years ago

Sorry guys, this fell off my radar.. I'll try to get it landed this week

	Shing Lyu [:shinglyu]
	Comment 15 • 9 years ago

Sorry for nagging, any update?

	Gregor Wagner [:gwagner]
	Comment 16 • 9 years ago

Any update here?

	Marshall Culpepper [:marshall_law] Assignee
	Comment 18 • 9 years ago

Comment on attachment 8599533 [details] [review]
[gaia] marshall:bug1134998_aumPrivateWindows > mozilla-b2g:master

This should ensure that no events are collected for private browser windows..

	David Flanagan [:djf]
	Comment 19 • 9 years ago

Comment on attachment 8599533 [details] [review]
[gaia] marshall:bug1134998_aumPrivateWindows > mozilla-b2g:master

I think I found one error in your tests. And I had a couple of other suggestions on github. But overall this code is simple and pretty clearly correctly (Assuming that isPrivateApp() does what it should... I don't know anything at all about that API.)

	Marshall Culpepper [:marshall_law] Assignee
	Comment 20 • 9 years ago

Thanks for the suggestions, David! I've pushed my changes up and hope to have it landed in master shortly

	Marshall Culpepper [:marshall_law] Assignee
	Comment 21 • 9 years ago

I'll be pushing up a v2.2 PR and requesting approval once this has landed in master as well

	Etienne Segonzac (:etienne) Reporter
	Comment 22 • 9 years ago

Comment on attachment 8599533 [details] [review]
[gaia] marshall:bug1134998_aumPrivateWindows > mozilla-b2g:master

thanks!

	Marshall Culpepper [:marshall_law] Assignee
	Comment 24 • 9 years ago

Comment on attachment 8601626 [details] [review]
[gaia] marshall:bug1134998_aumPrivateWindows_v2.2 > mozilla-b2g:v2.2

This disables any sort of app usage data collection for private browsers. Before this patch, we were counting the minutes of use for the private browser under the same manifest URL as the browser app (search.gaiamobile..), which made the metrics hard to discern. With this patch, no usage app usage should be counted in the private browser, whether or not it's under the heading of 'search.gaiamobile..'

[Bug caused by] Bug 982663
[User impact] if declined: Usage time of private browser will be recorded, but will be indistinguishable from the standard browser app
[Testing completed]: unit tests, flame
[Risk to taking this patch] (and alternatives if risky): 
[String changes made]: none

	bhavana bajaj [:bajaj]
	Comment 25 • 9 years ago

Still waiting for this to land on master.

Also observed the risk isn't specified, but looking at the patch it looks low risk enough, can you confirm?

	bhavana bajaj [:bajaj]
	Comment 26 • 9 years ago

Also, the try run does not look happy, can you help check if that's known issue and not tests failures?

	Marshall Culpepper [:marshall_law] Assignee
	Comment 27 • 9 years ago

Argh, forgot to post this
master: https://github.com/mozilla-b2g/gaia/commit/d4a2f8785c69b78471155189fb7f891b6f50c602

	Marshall Culpepper [:marshall_law] Assignee
	Comment 28 • 9 years ago

it looks like all the test jobs were failing to load somehow, I'm going to rebase and trigger a fresh run

	Marshall Culpepper [:marshall_law] Assignee
	Comment 29 • 9 years ago

The failures here seem to be something going on with the runner, each GU test is retried 5 times before it ultimately fails, but the failure log doesn't mention anything about the actual unit tests.

	Josh Cheng [:josh]
	Comment 30 • 9 years ago

Comment on attachment 8601626 [details] [review]
[gaia] marshall:bug1134998_aumPrivateWindows_v2.2 > mozilla-b2g:v2.2

Approving this patch for landing on 2.2. PLease NI if the try run infra issue do not sort out and we have to back this out.

	Ryan VanderMeulen [:RyanVM]
	Comment 31 • 9 years ago

v2.2: https://github.com/mozilla-b2g/gaia/commit/e048df68f6f4853b5826a8816e143d95258149de