898743 - 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

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

https://tbpl.mozilla.org/php/getParsedLog.php?id=25805782&tree=Fx-Team
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

Comment 1

[Legacy TBPL/Treeherder Robot]

RyanVM
https://tbpl.mozilla.org/php/getParsedLog.php?id=25888809&tree=Mozilla-Inbound
Windows XP 32-bit mozilla-inbound opt test mochitest-browser-chrome on 2013-07-29 17:54:12
revision: fed05531f9e3
slave: t-xp32-ix-092

18:22:00  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/thumbnails/test/browser_thumbnails_update.js | got notification of item being created. - Got 0, expected 1
18:22:00  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/thumbnails/test/browser_thumbnails_update.js | still only 1 notification of item being created. - Got 0, expected 1
18:22:00  WARNING -  TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/thumbnails/test/browser_thumbnails_update.js | c:\docume~1\cltbld~1.t-x\locals~1\temp\tmpqidjem\thumbnails\12a24d9c14efa10e597232154c375aca.png doesn't exist

Comment 2

[Legacy TBPL/Treeherder Robot]

philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=26287688&tree=Mozilla-Inbound
Rev3 Fedora 12 mozilla-inbound debug test mochitest-browser-chrome on 2013-08-07 17:19:01
revision: 49b181b3d59c
slave: talos-r3-fed-056

ImportError: cannot import name OrderedDict
TypeError: this._windows[aWindow.__SSi] is undefined
TypeError: profile is undefined: Sidebar.prototype<.getItemByProfile@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/sidebar.js:91
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

Comment 3

[Legacy TBPL/Treeherder Robot]

Tomcat
https://tbpl.mozilla.org/php/getParsedLog.php?id=26336664&tree=Mozilla-Inbound
Rev3 Fedora 12x64 mozilla-inbound debug test mochitest-browser-chrome on 2013-08-08 19:02:02
revision: 57b34c38a191
slave: talos-r3-fed64-065

ImportError: cannot import name OrderedDict
TypeError: profile is undefined: Sidebar.prototype<.getItemByProfile@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/sidebar.js:91
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

Comment 4

[Legacy TBPL/Treeherder Robot]

ttaubert
https://tbpl.mozilla.org/php/getParsedLog.php?id=26383499&tree=Fx-Team
Rev3 Fedora 12x64 fx-team debug test mochitest-browser-chrome on 2013-08-09 18:42:15
revision: 6f731cbd7303
slave: talos-r3-fed64-066

ImportError: cannot import name OrderedDict
TypeError: this._windows[aWindow.__SSi] is undefined
TypeError: profile is undefined: Sidebar.prototype<.getItemByProfile@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/profiler/sidebar.js:91
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

Comment 5

[Legacy TBPL/Treeherder Robot]

philor
https://tbpl.mozilla.org/php/getParsedLog.php?id=26944103&tree=Mozilla-Inbound
Windows XP 32-bit mozilla-inbound debug test mochitest-browser-chrome on 2013-08-23 15:04:10
revision: b44c22c84132
slave: t-xp32-ix-016

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/components/thumbnails/test/browser_thumbnails_update.js | we still have a green thumbnail - Got 255,0,0, expected 0,255,0

Comment 6

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-898743-don-t-thumbnail-non-http-channels-with-a-.patch

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.

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 795801 [details] [diff] [review]
0001-Bug-898743-don-t-thumbnail-non-http-channels-with-a-.patch

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.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-898743-ensure-Cache-Control-no-store-for-all-thu.patch

As suggested, just use the .sjs file instead of a data: URL for the test that goes orange.

Comment 9

[Drew Willcoxon :adw]

Comment on attachment 796990 [details] [diff] [review]
0001-Bug-898743-ensure-Cache-Control-no-store-for-all-thu.patch

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.

Comment 10

[Mark Hammond [:markh] [:mhammond]]

Thanks!  Pushed with comments addressed.

https://hg.mozilla.org/integration/fx-team/rev/ed58ee0f5829

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ed58ee0f5829

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider backporting to Aurora? Looks like it would need some adapting, though.

Comment 13

[Mark Hammond [:markh] [:mhammond]]

Attachment: Aurora version of the same patch

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

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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!

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0d081e6fa456