Closed
Bug 1397257
Opened 7 years ago
Closed 7 years ago
[Windows] Awesome Screenshot removing error for a second uninstallation
Categories
(WebExtensions :: Request Handling, defect, P1)
Tracking
(firefox-esr52 unaffected, firefox55 unaffected, firefox56 verified, firefox57 verified)
VERIFIED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | verified |
firefox57 | --- | verified |
People
(Reporter: vtamas, Assigned: haik)
References
Details
(Keywords: regression)
Attachments
(4 files)
[Affected versions]: Firefox 57.0a1 (20170905220108) [Affected platforms]: Windows 10 64-bit Windows 7 64-bit [Steps to reproduce]: 1.Launch Firefox Release with a clean profile. 2.Navigate to Add-ons Manager -> Get Add-ons and install Awesome Screenshot webextension. 3.Click on the switch button and remove the webextension. 4.Install the webextension one more time. 5.Remove it again. [Expected Results]: The webextensions is successfully uninstalled and there are no errors encountered. [Actual Results]: - “An unexpected error occurred during uninstallation.” error is displayed in Disco Pane - The following errors are thrown in Browser Console” https://pastebin.com/jPApVsNH . - The webextensions still appears as installed in “Extensions” tab but the icon is no longer displayed in toolbar. - This issue also reproduces while installing the webextension from amo. See attached screencast. [Additional notes]: - This issue is not reproducible under Ubuntu 16.04 32-bit and Mac OS X 10.12.3.
Assignee | ||
Comment 1•7 years ago
|
||
Vasilica, could you try to get a regression window?
Flags: needinfo?(vasilica.mihasca)
Updated•7 years ago
|
Flags: needinfo?(aswan)
Reporter | ||
Comment 2•7 years ago
|
||
(In reply to Haik Aftandilian [:haik] from comment #1) > Vasilica, could you try to get a regression window? Sorry, I forgot to mention that this issue was caused by Bug 1395898. Last good revision: 6106d550ba968098cfb69a427c6dd646935a58d9 First bad revision: e12897c25fc597353f9ad42c9cc185a41fa9001f Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6106d550ba968098cfb69a427c6dd646935a58d9&tochange=e12897c25fc597353f9ad42c9cc185a41fa9001f It is not reproducible with extensions.webextensions.remote set to false.
Flags: needinfo?(vasilica.mihasca)
Assignee | ||
Comment 3•7 years ago
|
||
I don't know the root cause yet, but I think there is a problem that has always existed with OOP extensions. The JAR caching that was turned on in bug 1376496 seems to mask the problem. I tried going back to earlier builds on Windows with OOP extensions enabled. On Nightly 2017-07-17 on Windows where extensions.webextensions.remote=true, uninstalling Awesome Screenshot succeeds but the icon remains in the task bar. That was before This may be unrelated, but every version I tried triggers "failed to remove temporary file C:\Users\<USERNAME>\AppData\Local\Temp\tmp-<RANDOM>.xpi" during Awesome Screenshot installation: 1504807266088 addons.xpi WARN Failed to remove temporary file C:\Users\HAIKAF~1\AppData\Local\Temp\tmp-z0j.xpi for addon https://addons.mozilla.org/firefox/downloads/file/669836/awesome_screenshot_plus_capture_annotate_more-3.0.21-an+fx.xpi?src=discovery-promo: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.remove]" nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)" location: "JS frame :: resource://gre/modules/addons/XPIInstall.jsm :: removeTemporaryFile :: line 1558" data: no] Stack trace: removeTemporaryFile()@resource://gre/modules/addons/XPIInstall.jsm:1558 < startInstall/<()@resource://gre/modules/addons/XPIInstall.jsm:1907
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → haftandilian
Assignee | ||
Comment 4•7 years ago
|
||
Correcting my cut-off comment above: (In reply to Haik Aftandilian [:haik] from comment #3) > I tried going back to earlier builds on Windows with OOP extensions enabled. > On Nightly 2017-07-17 on Windows where extensions.webextensions.remote=true, > uninstalling Awesome Screenshot succeeds but the icon remains in the task > bar. That was before That was before bug 1376496 integrated.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a853ce40675d28c64349f55438e48228d3af41bf
Assignee | ||
Comment 7•7 years ago
|
||
Vasilica, could you test with the try build and see if it resolves the problem for you? Here's a direct link to the non-debug windows build. https://queue.taskcluster.net/v1/task/e3HOHcB7TbGUhoZ6sVBdpw/runs/0/artifacts/public/build/target.zip
Flags: needinfo?(vasilica.mihasca)
Reporter | ||
Comment 8•7 years ago
|
||
(In reply to Haik Aftandilian [:haik] from comment #7) > Vasilica, could you test with the try build and see if it resolves the > problem for you? Here's a direct link to the non-debug windows build. > > https://queue.taskcluster.net/v1/task/e3HOHcB7TbGUhoZ6sVBdpw/runs/0/ > artifacts/public/build/target.zip This issue is no longer reproducible while using the try-build under Windows 10 64-bit and Windows 7 64-bit. The webextension is enabled/disabled without error. See screencast.
Flags: needinfo?(vasilica.mihasca)
Assignee | ||
Comment 9•7 years ago
|
||
Thanks, Vasilica. I need to spend some more time making sure the fix is correct and then I'll post for review.
Assignee | ||
Comment 10•7 years ago
|
||
The problem here is also related to the handling of nonexistent JAR paths (that is, a path to an inner file that does not exist in the JAR) and it is a bug in the fix for bug 1395898. In GetAsync() when we are handling a cached JAR and mJarChannel->AsyncOpen2(mListener) fails, we return the error to SimpleChannel::BeginAsyncRead() which exits early without clearing mCallbacks. SimpleChannel.mCallbacks holds an owning reference to the ExtensionStreamGetter. ExtensionStreamGetter holds an owning reference to the SimpleChannel and the cached JAR channel. As a result, we return from SimpleChannel::BeginAsyncRead() with a circular dependency relationship between SimpleChannel and ExtensionStreamGetter. That prevents the ExtensionStreamGetter from being freed which means the cached JAR channel is never freed. The cached JAR channel holds an owning reference to an nsIZipReader for the cached JAR (to ensure it isn't displaced from the JAR cache before we read from the JAR.) Holding the reference to the nsIZipReader prevents it from being freed when the JAR cache is cleared. Which also prevents the underlying file descriptor from being closed. The messages on the developer console indicate the extension files can't be moved out of the profile. I haven't traced through the code to see exactly how failing to close the JAR triggers the uninstall failure. It is likely due to the parent having opened the file without using the ShareDelete flag. Still looking into that part of this.
Flags: needinfo?(aswan)
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8905777 [details] Bug 1397257 - [Windows] Awesome Screenshot removing error for a second uninstallation. https://reviewboard.mozilla.org/r/177580/#review183834
Attachment #8905777 -
Flags: review?(jmathies) → review+
Comment 13•7 years ago
|
||
Pushed by haftandilian@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/74ab1f8f99bb [Windows] Awesome Screenshot removing error for a second uninstallation. r=jimm
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/74ab1f8f99bb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Comment 15•7 years ago
|
||
Comment on attachment 8905777 [details] Bug 1397257 - [Windows] Awesome Screenshot removing error for a second uninstallation. Approval Request Comment [Feature/Bug causing the regression]: Bug 1395898 [User impact if declined]: Extensions may fail to uninstall. [Is this code covered by automated tests?]: Partially. Not all the error paths are covered. [Has the fix been verified in Nightly?]: Will be in the next Nightly build today. [Needs manual test from QE? If yes, steps to reproduce]: Yes. Follow Vasilica's steps on this bug description. [List of other uplifts needed for the feature/fix]: This fix depends on 1395898. [Is the change risky?]: No [Why is the change risky/not risky?]: Changes are isolated to extension handling and fix a regression. Changes are pretty well understood and frequently executed. [String changes made/needed]: None
Attachment #8905777 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 16•7 years ago
|
||
Marking this affected now that we intend to uplift the patch from bug 1395898
Comment 17•7 years ago
|
||
Comment on attachment 8905777 [details] Bug 1397257 - [Windows] Awesome Screenshot removing error for a second uninstallation. Fix for some install/uninstall situations for webextensions. OK to uplift for beta 12
Attachment #8905777 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 18•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/fafd69ca2733
Updated•7 years ago
|
Flags: qe-verify+
Reporter | ||
Comment 19•7 years ago
|
||
I confirm that the Awesome Screenshot extension is successfully uninstalled and there are no errors encountered on Firefox 57.0a1 (20170915100121) and Firefox 56.0b12 (20170914024831) under Windows 10 64-bit and Windows 7 64-bit. See screencast.
Reporter | ||
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•