Closed
Bug 1395898
Opened 7 years ago
Closed 7 years ago
[OOP] jar cache incorrectly handled non-existent file paths
Categories
(WebExtensions :: Request Handling, defect)
WebExtensions
Request Handling
Tracking
(firefox-esr52 unaffected, firefox55 unaffected, firefox56 fixed, firefox57 verified)
VERIFIED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | fixed |
firefox57 | --- | verified |
People
(Reporter: vtamas, Assigned: haik)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(3 files)
21.24 KB,
image/png
|
Details | |
59 bytes,
text/x-review-board-request
|
jimm
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
4.56 MB,
image/gif
|
Details |
[Affected versions]: Firefox 57.0a1 (2017-08-31) Firefox 56.0b8 (20170831165232) [Affected platforms]: Windows 10 64-bit Mac OS X 10.12.3 [Steps to reproduce]: 1.Launch Firefox with a clean profile. 2.Make sure that "extensions.webextensions.remote” pref is set to true. 3.Install the following webextension: https://addons.mozilla.org/ro/firefox/addon/screenshot-capture-annotate/?src=ss 4.Navigate to a new webpage (e.g. https://www.amazon.com/) 5.Click on the webextension icon from toolbar. 6.Select any option from the list. [Expected Results]: The webextension options work without any issue. [Actual Results]: - Capture tabs are never loaded. - This issue is not reproducible with "extensions.webextensions.remote” pref is set to false. - The following error appears in Browser Console: Unchecked lastError value: Error: Could not establish connection. Receiving end does not exist. ExtensionCommon.jsm:407 - See attached screenshot. [Regression Range]: Last good revision: 29a2d717c525f7f50ed5d7b7afe2aa25a96a6bbd First bad revision: 9878f96c556e62c4253c2fec24e156309a29a1ac Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=29a2d717c525f7f50ed5d7b7afe2aa25a96a6bbd&tochange=9878f96c556e62c4253c2fec24e156309a29a1ac Looks like the following bug has the changes which introduced the regression: Bug 1390346 [Additional notes]: The extension is not uninstalled after clicking on “Remove” button from about:addons.
Comment 1•7 years ago
|
||
@haik: Not much to go on here yet, but certainly seems to be jar caching based on the testing i did. Any thoughts?
Flags: needinfo?(haftandilian)
Assignee | ||
Comment 2•7 years ago
|
||
(In reply to Shane Caraveo (:mixedpuppy) from comment #1) > @haik: Not much to go on here yet, but certainly seems to be jar caching > based on the testing i did. Any thoughts? I was looking at this yesterday and couldn't make sense of it, but I just mis-typed a moz-extension:// URI and realized the failure for a non-existent file is the same as what is happening here. The problem is that the code I added to use SimpleChannel for remote moz-extension URI's that resolve to a cached JAR has a bug (a silly careless mistake) in the error handling: the return value from AsyncOpen2() is being ignored. That breaks this particular extension because the extension's edit.html file includes a script ("javascripts/libs/jquery.ui.resizable.min.js") that is missing from the XPI. Removing the offending <script ...> line from edit.html makes things work again. That's <script type="text/javascript" src="javascripts/libs/jquery.ui.resizable.min.js"></script> The issue with the fix for bug 1390346 is that the return value from "mJarChannel->AsyncOpen2(mListener)" for the cache hit case in ExtensionStreamGetter::GetAsync() is being ignored. Ignoring the AsyncOpen2() failure is causing the load to hang.
Flags: needinfo?(haftandilian)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → haftandilian
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
Try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83394d38a707dabb32af5859d6a7059d02cd1442 https://treeherder.mozilla.org/#/jobs?repo=try&revision=34971b08f2c8979d50516d9ce9454009b9e0d978
Comment 5•7 years ago
|
||
Thanks Haik!
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8904040 [details] Bug 1395898 - Awesome Screenshot webextension does not work. https://reviewboard.mozilla.org/r/175768/#review180818
Attachment #8904040 -
Flags: review?(jmathies) → review+
Pushed by haftandilian@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e12897c25fc5 Awesome Screenshot webextension does not work. r=jimm
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e12897c25fc5
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 9•7 years ago
|
||
Haik, can you please request uplift to 56 when you get a chance? Andreas, can you contact the Awesome Screenshots devs about their attempts to load a nonexistent script? Thanks.
Flags: needinfo?(haftandilian)
Flags: needinfo?(awagner)
Assignee | ||
Comment 10•7 years ago
|
||
Comment on attachment 8904040 [details] Bug 1395898 - Awesome Screenshot webextension does not work. Approval Request Comment [Feature/Bug causing the regression]: The fix for bug 1390346 introduced this regression. [User impact if declined]: Some extension pages may fail to load--they'll be stuck "loading" and never complete. [Is this code covered by automated tests?]: This particular failure is not, but the area being changed is well covered. [Has the fix been verified in Nightly?]: I've verified this is fixed in today's (2017-09-05) Nightly. [Needs manual test from QE? If yes, steps to reproduce]: QE should verify the Beta fix. Use the current version 3.0.21 of the Awesome Screenshot extension from the description (newer versions might get a fix that avoids the problem.) [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: This is a small followup fix to add a missing check of a return value. The change is similar to what is already used in ExtensionProtocolHandler.cpp for other cases. [String changes made/needed]: None
Flags: needinfo?(haftandilian)
Attachment #8904040 -
Flags: approval-mozilla-beta?
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 11•7 years ago
|
||
Hi Vasilica, Can you help check if this issue was fixed in the latest nightly?
Flags: needinfo?(vasilica.mihasca)
Updated•7 years ago
|
Flags: qe-verify+
Reporter | ||
Comment 13•7 years ago
|
||
Verified on Firefox 57.0a1 (20170905220108) under Windows 10 64-bit and Mac OS X 10.12.3 and the webextensions pages are successfully loaded but the following errors are still displayed in browser console: Unchecked lastError value: Error: Could not establish connection. Receiving end does not exist. ExtensionCommon.jsm:407 Error: Response handle went out of scope undefined Unchecked lastError value: Error: Response handle went out of scope ExtensionCommon.jsm:407 this.widget is null ext-browserAction.js:523 Any thoughts about these errors? Also a new regression was encountered on Windows when removing the Awesome Screenshot webextensions for a second time: Bug 1397257
Flags: needinfo?(vasilica.mihasca)
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(haftandilian)
Assignee | ||
Comment 14•7 years ago
|
||
I'm not familiar with the JS side of extension loading. Shane, is that something you could take a look at?
Flags: needinfo?(haftandilian) → needinfo?(mixedpuppy)
Comment 15•7 years ago
|
||
(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #13) > Created attachment 8905033 [details] > awesomescreenshot.gif > > Verified on Firefox 57.0a1 (20170905220108) under Windows 10 64-bit and Mac > OS X 10.12.3 and the webextensions pages are successfully loaded but the > following errors are still displayed in browser console: > Unchecked lastError value: Error: Could not establish connection. > Receiving end does not > exist. ExtensionCommon.jsm:407 > Error: Response handle went out of scope undefined > Unchecked lastError value: Error: Response handle went out of scope > ExtensionCommon.jsm:407 > this.widget is null ext-browserAction.js:523 > > Any thoughts about these errors? These may or may not be related and I'm betting not since these have nothing to do with file access. Lets have another bug on that. > Also a new regression was encountered on Windows when removing the Awesome > Screenshot webextensions for a second time: Bug 1397257 Is this regression due to the patch in this bug? If not please don't block this bug on it.
Summary: Awesome Screenshot webextension does not work → [OOP] jar cache incorrectly handled non-existent file paths
Comment 16•7 years ago
|
||
(In reply to Haik Aftandilian [:haik] from comment #14) > I'm not familiar with the JS side of extension loading. Shane, is that > something you could take a look at? I ni? aswan on the bug created for that issue.
Flags: needinfo?(mixedpuppy)
Reporter | ||
Comment 17•7 years ago
|
||
(In reply to Shane Caraveo (:mixedpuppy) from comment #15) > These may or may not be related and I'm betting not since these have nothing > to do with file access. Lets have another bug on that. Filed Bug 1397730 > Is this regression due to the patch in this bug? If not please don't block > this bug on it. Yes. Bug 1397257 was regressed by this one. Based on Comment 13 and Comment 15 I am marking this bug as Verified fixed on Firefox 57.
Status: RESOLVED → VERIFIED
Comment 18•7 years ago
|
||
Comment on attachment 8904040 [details] Bug 1395898 - Awesome Screenshot webextension does not work. Fix a regression and was verified. Beta56+.
Attachment #8904040 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee | ||
Comment 19•7 years ago
|
||
I'd like to hold off on uplifting this to beta until bug 1397257 is fixed.
Comment 20•7 years ago
|
||
Comment on attachment 8904040 [details] Bug 1395898 - Awesome Screenshot webextension does not work. Per comment #19, hold off on uplifting to Beta56.
Attachment #8904040 -
Flags: approval-mozilla-beta+ → approval-mozilla-beta-
Assignee | ||
Comment 21•7 years ago
|
||
Now that bug 1397257 is fixed, I'd like to re-request we uplift this to Beta. Uplift of bug 1397257 is also required for this.
Flags: needinfo?(gchang)
Comment 22•7 years ago
|
||
Comment on attachment 8904040 [details] Bug 1395898 - Awesome Screenshot webextension does not work. Fix for a regression from 56 that may prevent some extensions from loading. This should uplift along with the fix from bug 1397257.
Attachment #8904040 -
Flags: approval-mozilla-beta- → approval-mozilla-beta+
Comment 23•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/58e1249a4920
Updated•7 years ago
|
Flags: needinfo?(gchang)
Updated•6 years ago
|
Product: Toolkit → WebExtensions
You need to log in
before you can comment on or make changes to this bug.
Description
•