Closed
Bug 892486
Opened 12 years ago
Closed 12 years ago
Add telemetry for appcache
Categories
(Core :: Networking: Cache, defect)
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: ehsan.akhgari, Assigned: mayhemer)
Details
(Whiteboard: [qa-])
Attachments
(1 file)
1.96 KB,
patch
|
mcmanus
:
review+
bajaj
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
mayhemer
:
checkin+
|
Details | Diff | Splinter Review |
We need to know more about how many websites are using appcache.
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → honzab.moz
Reporter | ||
Comment 1•12 years ago
|
||
We need to collect the ratio of appcached document loads versus non-appcached document loads.
![]() |
Assignee | |
Comment 2•12 years ago
|
||
- for top-level document loads it records boolean probe
- when appcache is found for the URL it comes from the appcache, otherwise not
Attachment #775862 -
Flags: review?(ehsan)
Reporter | ||
Comment 3•12 years ago
|
||
Comment on attachment 775862 [details] [diff] [review]
v1
Review of attachment 775862 [details] [diff] [review]:
-----------------------------------------------------------------
I'm not very well versed in these telemetry probes. Patrick, could you please do the honors here?
Attachment #775862 -
Flags: review?(ehsan) → review?(mcmanus)
Comment 4•12 years ago
|
||
Comment on attachment 775862 [details] [diff] [review]
v1
Review of attachment 775862 [details] [diff] [review]:
-----------------------------------------------------------------
::: netwerk/protocol/http/nsHttpChannel.cpp
@@ +2547,5 @@
> NS_ENSURE_SUCCESS(rv, rv);
> }
> }
>
> + if (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI)
one line: if (foo) {
Attachment #775862 -
Flags: review?(mcmanus) → review+
![]() |
Assignee | |
Comment 5•12 years ago
|
||
Comment on attachment 775862 [details] [diff] [review]
v1
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c2d7cf35605
Attachment #775862 -
Flags: checkin+
Reporter | ||
Comment 6•12 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #5)
> Comment on attachment 775862 [details] [diff] [review]
> v1
>
> https://hg.mozilla.org/integration/mozilla-inbound/rev/6c2d7cf35605
Technically it should've been r=mcmanus. :-)
![]() |
Assignee | |
Comment 7•12 years ago
|
||
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #6)
> (In reply to Honza Bambas (:mayhemer) from comment #5)
> > Comment on attachment 775862 [details] [diff] [review]
> > v1
> >
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/6c2d7cf35605
>
> Technically it should've been r=mcmanus. :-)
Looks like I've turned that upside down :)) Sorry!
![]() |
Assignee | |
Comment 8•12 years ago
|
||
As I'm thinking of it, we might want to add one more probe.
The probe in the patch only measures what top-level pages we load from appcache. But we don't measure what pages are actually "using appcache" by means of defining the manifest attribute.
Since we have the prompt and there is an option "Never for This Site" some people could turn to never use appcache. We don't find out this as well as users just close or ignore the prompt at all.
So, we may need another probe like "HAS_APPCACHE_MANIFEST" added to the parser.
Opinions?
Reporter | ||
Comment 9•12 years ago
|
||
Sounds like a good idea to me!
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment 11•12 years ago
|
||
uplift to m-a, right?
![]() |
Assignee | |
Comment 12•12 years ago
|
||
Comment on attachment 775862 [details] [diff] [review]
v1
[Approval Request Comment]
Bug caused by (feature/regressing bug #): None
User impact if declined: None
Testing completed (on m-c, etc.): Just landed
Risk to taking this patch (and alternatives if risky): Zero
String or IDL/UUID changes made by this patch: None
Attachment #775862 -
Flags: approval-mozilla-aurora?
Reporter | ||
Comment 13•12 years ago
|
||
Any maybe beta? I think we should try to start to get telemetry data ASAP.
Updated•12 years ago
|
Attachment #775862 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
![]() |
Assignee | |
Comment 14•12 years ago
|
||
Comment on attachment 775862 [details] [diff] [review]
v1
[Approval Request Comment]
same as for aurora
Attachment #775862 -
Flags: approval-mozilla-beta?
Updated•12 years ago
|
Attachment #775862 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•12 years ago
|
||
Comment 16•12 years ago
|
||
Do we know of particular websites or use cases where appcache is used so QA can verify the telemetry probes are working?
Comment 17•12 years ago
|
||
Anthony, don't worry about verification of this bug. There are far more important things to test.
![]() |
Assignee | |
Comment 19•12 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #8)
> So, we may need another probe like "HAS_APPCACHE_MANIFEST" added to the
> parser.
If we decide to remove the prompt soon, I think there is no need for this additional probe. I'll wait until that bug is solved or put to ice.
You need to log in
before you can comment on or make changes to this bug.
Description
•