Closed Bug 898743 Opened 7 years ago Closed 7 years ago

Intermittent browser_thumbnails_update.js | got notification of item being created. - Got 2, expected 1 | still only 1 notification of item being created. - Got 2, expected 1


(Firefox :: Tabbed Browser, defect)

Not set



Firefox 26
Tracking Status
firefox24 --- unaffected
firefox25 --- fixed
firefox26 --- fixed


(Reporter: ttaubert, Assigned: markh)


(Keywords: intermittent-failure)


(2 files, 1 obsolete file)
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/thumbnails/test/browser_thumbnails_update.js | got notification of item being created. - Got 2, expected 1
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/thumbnails/test/browser_thumbnails_update.js | still only 1 notification of item being created. - Got 2, expected 1
I think the problem here is that our data: URL is being captured by the f/g service.  We've fixed that for tests that use the .sjs responses by setting the cache-control header.

This patch checks if a non-http channel still has HTML, and if so, queries for a http-equiv cache-control tag, and if no-store is in that, skips the capture.  This test also adds that tag.
Assignee: nobody → mhammond
Attachment #795801 - Flags: review?(adw)
Comment on attachment 795801 [details] [diff] [review]

adw pointed out on IRC that maybe we should just use the .sjs file in that test too, and avoid adding the extra checks to browser-thumbnail, which is hard to argue with.  I'll wait and see how he goes with bug 901294 as a fix may also come out of that, otherwise I'll do a new patch over the next few days.
Attachment #795801 - Attachment is obsolete: true
Attachment #795801 - Flags: review?(adw)
As suggested, just use the .sjs file instead of a data: URL for the test that goes orange.
Attachment #796990 - Flags: review?(adw)
Comment on attachment 796990 [details] [diff] [review]

Review of attachment 796990 [details] [diff] [review]:

::: toolkit/components/thumbnails/test/thumbnails_update.sjs
@@ +24,5 @@
>    aResponse.setHeader("Cache-Control", "no-store");
> +  // for the simple test - just return some content.
> +  if (aRequest.queryString == "simple") {
> +    aResponse.write("<html><body bgcolor=ff0000></body></html>");

The !simple case returns red and green pages, so please return a white page here (e.g., by not specifying a color at all) so that we don't confuse ourselves when we're debugging problems.  Not to mention, if there's some bug in a test that's expecting a !simple page but it ends up getting the simple page -- such a test should fail cleanly.

simpleCaptureTest says "Create a tab with a red background," but it doesn't actually matter what the background is.
Attachment #796990 - Flags: review?(adw) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Is this something we should consider backporting to Aurora? Looks like it would need some adapting, though.
Flags: needinfo?(mhammond)
This is a test only version of the patch for Aurora.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 870100
User impact if declined: Oraange-factor
Testing completed (on m-c, etc.): Landed on m-c
Risk to taking this patch (and alternatives if risky): Low risk, test-only patch.
String or IDL/UUID changes made by this patch: None
Attachment #797584 - Flags: review+
Attachment #797584 - Flags: approval-mozilla-aurora?
Flags: needinfo?(mhammond)
Comment on attachment 797584 [details] [diff] [review]
Aurora version of the same patch

Test-only patches don't need approval. I'll push this soon, thanks!
Attachment #797584 - Flags: approval-mozilla-aurora?
You need to log in before you can comment on or make changes to this bug.