Closed Bug 1316394 Opened 8 years ago Closed 2 years ago

Intermittent toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_saveAs.html | file was not downloaded

Categories

(WebExtensions :: General, defect, P3)

Product:
WebExtensions
Component:
General
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED INACTIVE

People

(Reporter: intermittent-bug-filer, Assigned: robwu)

References

(Depends on 1 open bug)

Details

(Keywords: bulk-close-intermittents, intermittent-failure, Whiteboard: [stockwell unknown])

Component: WebExtensions: Untriaged → WebExtensions: General
Priority: -- → P3
Status: NEW → RESOLVED
Closed: 7 years ago
Keywords: bulk-close-intermittents
Resolution: --- → INCOMPLETE
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
https://wiki.mozilla.org/Bug_Triage#Intermittent_Test_Failure_Cleanup
Status: REOPENED → RESOLVED
Closed: 7 years ago → 6 years ago
Resolution: --- → INCOMPLETE
Recent failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=163903007&repo=mozilla-inbound&lineNumber=7142
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
https://wiki.mozilla.org/Bug_Triage#Intermittent_Test_Failure_Cleanup
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → INCOMPLETE
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---
Product: Toolkit → WebExtensions

Shane, any idea what's going wrong here? Is the remove operation of the file saved before not complete yet?

Flags: needinfo?(mixedpuppy)

The increase in frequency seems to coincide with bug 1395207 landing a patch.

Flags: needinfo?(rob)
Flags: needinfo?(mixedpuppy)
Flags: needinfo?(mcs)
See Also: → 1395207

(In reply to Shane Caraveo (:mixedpuppy) from comment #115)

The increase in frequency seems to coincide with bug 1395207 landing a patch.

I think that the increase in frequency is because the patch for bug 1395207 added a lot more test cases. Looking at the older and recent failures, it appears that within testExpectFilePicker() something goes wrong between:

pickerFile.remove(false);

and

ok(!pickerFile.exists(), "file was not downloaded");

I don't know what the root cause could be though, since the remove() call is synchronous. Is there a way to find out if the test failures are always on Windows?

Flags: needinfo?(mcs)

To see the list of failures and their platforms, go to the top of this page and expand the section "Details". There should be a "Orange Factor" graphic with a link to the right. Click that link.

As mentioned in comment 116, something goes wrong in this range: https://searchfox.org/mozilla-central/rev/dcd9c2d2bc19d96d487825eb70c2333a4d60994e/toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_saveAs.html#151-162
Specifically, after removing the file, it is unexpectedly recreated (on Windows).

In this code range, there is nothing that creates the file, so I guess that the file is (re)created by the download request from a few lines back (line 146).
The done message from line 158 is in response to the downloads.onChanged event with state="complete" (via ext-downloads.js), which is ultimately triggered via _notifyChange near the end Download's start method.

After _notifyChange(), there is a _succeed() call, which calls DownloadIntegration.downloadDone(), which does a bunch of stuff including writing an Alternative Data Stream to the file (which implicitly creates the file if it had been deleted): https://searchfox.org/mozilla-central/rev/227f22acef5c4865503bde9f835452bf38332c8e/toolkit/components/downloads/DownloadIntegration.jsm#589,611

I manually tested this by putting a breakpoint at the start of downloadDone, calling browser.downloads.download from an extension with a blob:-URL and dummy file name. When the file was downloaded, the breakpoint was triggered. I deleted the file in Explorer, resumed the execution and an empty file was created again.

Flags: needinfo?(rob)
Depends on: 1654819

In the last 7 days this bug failed 25 times. Happens on windows10-64, windows10-64-shippable, windows7-32 mostly opt build type.

Recent log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=312420059&repo=autoland&lineNumber=6351

Whiteboard: [stockwell needswork:owner]

Ideally bug 1654819 should be fixed.

If it doesn't then I would prefer monkey-patching https://searchfox.org/mozilla-central/rev/50cb0892948fb4291b9a6b1b30122100ec7d4ef2/toolkit/components/downloads/DownloadIntegration.jsm#520 to return false (to disable the logic at https://searchfox.org/mozilla-central/rev/50cb0892948fb4291b9a6b1b30122100ec7d4ef2/toolkit/components/downloads/DownloadIntegration.jsm#598 ) over disabling the test (I guess that would be the next step after "stockwell needswork:owner"? I'm already watching this bug anyway, so I guess that I own the bug?)

(In reply to Rob Wu [:robwu] from comment #131)

Ideally bug 1654819 should be fixed.

If it doesn't then I would prefer monkey-patching https://searchfox.org/mozilla-central/rev/50cb0892948fb4291b9a6b1b30122100ec7d4ef2/toolkit/components/downloads/DownloadIntegration.jsm#520 to return false (to disable the logic at https://searchfox.org/mozilla-central/rev/50cb0892948fb4291b9a6b1b30122100ec7d4ef2/toolkit/components/downloads/DownloadIntegration.jsm#598 ) over disabling the test (I guess that would be the next step after "stockwell needswork:owner"? I'm already watching this bug anyway, so I guess that I own the bug?)

stockwell needswork:owner - mostly refers to the triage owner and since you're watching/working on this I assume you're the assignee?
We disable the tests when they reach a 150 failures in the last 30 days and until then, poke people on the bugs.

I'm assigning the bug to myself because I know why it's happening and how it can be fixed (either a real fix via bug 1654819 or test-only bandaid according to comment 131).

Feel free to poke me to raise the priority if the frequency of failures become unpalatable.

Assignee: nobody → rob

(In reply to Rob Wu [:robwu] from comment #134)

I'm assigning the bug to myself because I know why it's happening and how it can be fixed (either a real fix via bug 1654819 or test-only bandaid according to comment 131).

Feel free to poke me to raise the priority if the frequency of failures become unpalatable.

Thank you. For now the failure rate is low, but we'll ni if it increases.

There have been 34 total failures in the last 7 days:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?startday=2021-01-17&endday=2021-01-24&tree=trunk&bug=1316394
Affected platforms are:

  • windows7-32-shippable
  • windows7-32 opt & debug
  • windows10-64-shippable
  • windows10-64 opt & debug

Recent log: https://treeherder.mozilla.org/logviewer?job_id=327597949&repo=autoland&lineNumber=116786

Hi Rob, based on above comments, can you take a look?

Flags: needinfo?(rob)
Whiteboard: [stockwell unknown] → [stockwell needswork:owner]

I took a look (follow-up to comment 131 and comment 134) and posted my analysis plus proposed fix at https://bugzilla.mozilla.org/show_bug.cgi?id=1654819#c4

This week is reserved for other work (including a virtual All hands with my team), I'll save the needinfo for next week.

failure rate dropped again, I've reprioritized accordingly. If the rates jump again I'll reprioritize.

Flags: needinfo?(rob)
Severity: normal → S4

disabled on win10 in bug 1695612, otherwise been no activity for a while.

Status: REOPENED → RESOLVED
Closed: 6 years ago → 2 years ago
Resolution: --- → INACTIVE
See Also: → 1825957
You need to log in before you can comment on or make changes to this bug.