Closed Bug 1328686 Opened 3 years ago Closed 3 years ago

add some diagnostic asserts in dom/cache

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I'd like to use diagnostic assertions in more places.  Cache API seems like a good place to start.
In an effort to catch more bugs sooner, this converts all of the low-cost assertions in dom/cache to MOZ_DIAGNOSTIC_ASSERT().  Things that were O(n), required TLS lookup, etc I left as DEBUG-only assertions.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=117040ba614438a4d6a6fa63e2a9d2436d78f229
Attachment #8824476 - Flags: review?(bugmail)
Comment on attachment 8824476 [details] [diff] [review]
Add diagnostic assertions to dom/cache. r=asuth

Review of attachment 8824476 [details] [diff] [review]:
-----------------------------------------------------------------

In a corruption of the immortal words of Barry and LeVon, that's a lot of diagnostic asserts.
Attachment #8824476 - Flags: review?(bugmail) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/46cb4ee49786
Add diagnostic assertions to dom/cache. r=asuth
https://hg.mozilla.org/mozilla-central/rev/46cb4ee49786
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment on attachment 8824476 [details] [diff] [review]
Add diagnostic assertions to dom/cache. r=asuth

Approval Request Comment
[Feature/Bug causing the regression]: Generic service worker stability
[User impact if declined]: Purpose of this bug is to help us narrow down crashes in service worker code.  Does not address a specific problem, though.
[Is this code covered by automated tests?]: The purpose of the diagnostic assertions are to perform additional checks in non-DEBUG builds of dev-edition and nightly.  Any revealed problems will show up in crash-stats.
[Has the fix been verified in Nightly?]:  The assertions have been on nightly for a couple days.
[Needs manual test from QE? If yes, steps to reproduce]:   No
[List of other uplifts needed for the feature/fix]: No.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: The diagnostic assertions have no impact on release or beta.
[String changes made/needed]: None
Attachment #8824476 - Flags: approval-mozilla-aurora?
Comment on attachment 8824476 [details] [diff] [review]
Add diagnostic assertions to dom/cache. r=asuth

convert some dom/cache asserts to diagnostic asserts, aurora52+
Attachment #8824476 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This change is going to break debug builds on beta when it reaches there:

/home/worker/workspace/build/src/dom/cache/DBSchema.cpp:2561:90: error: 'lastVersion' was not declared in this scope
/home/worker/workspace/build/src/dom/cache/DBSchema.cpp:2561:102: error: template argument 1 is invalid
/home/worker/workspace/build/src/dom/cache/Manager.cpp:1533:100: error: 'oldRef' was not declared in this scope
/home/worker/workspace/build/src/dom/cache/Manager.cpp:1533:107: error: template argument 1 is invalid
/home/worker/workspace/build/src/dom/cache/Manager.cpp:1584:99: error: 'oldRef' was not declared in this scope
/home/worker/workspace/build/src/dom/cache/Manager.cpp:1584:106: error: template argument 1 is invalid
/home/worker/workspace/build/src/dom/cache/StreamControl.cpp:57:7: error: 'closedCount' was not declared in this scope
/home/worker/workspace/build/src/dom/cache/StreamControl.cpp:61:71: error: 'closedCount' was not declared in this scope
/home/worker/workspace/build/src/dom/cache/StreamControl.cpp:61:87: error: template argument 1 is invalid
It also breaks non-debug builds:

/home/worker/workspace/build/src/dom/cache/StreamControl.cpp:57:7: error: use of undeclared identifier 'closedCount'
Depends on: 1331949
Depends on: 1341206
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.