Last Comment Bug 866636 - AddonUpdateChecker bypasses it cache, but doesn't inhibit writing to it
: AddonUpdateChecker bypasses it cache, but doesn't inhibit writing to it
Status: RESOLVED FIXED
[good first bug][mentor=bmcbride@mozi...
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla23
Assigned To: Ritesh
:
: Andy McKay [:andym]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-29 03:06 PDT by Blair McBride [:Unfocused] (UNAVAILABLE)
Modified: 2013-05-06 04:32 PDT (History)
1 user (show)
blair: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Just added flag for INHIBIT_CACHING (951 bytes, patch)
2013-04-30 23:49 PDT, Ritesh
blair: review+
Details | Diff | Splinter Review

Description Blair McBride [:Unfocused] (UNAVAILABLE) 2013-04-29 03:06:24 PDT
Seems AddonUpdateChecker.jsm bypasses the cache when requesting the update manifest (using LOAD_BYPASS_CACHE), but doesn't inhibit writing to it. It should!

It sets LOAD_BYPASS_CACHE here:
http://hg.mozilla.org/mozilla-central/file/1c0e964bd620/toolkit/mozapps/extensions/AddonUpdateChecker.jsm#l414

It should also set INHIBIT_CACHING, like what is done in bug 866564 in LightweightThemeManager.jsm:
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=866564&attachment=742892
Comment 1 Ritesh 2013-04-30 06:04:39 PDT
I want to work on this bug. Do I need to just set the INHIBIT_CACHING flag (one line) or something else?

(In reply to Blair McBride [:Unfocused] (Back from the dead. Mostly.) from comment #0)
> Seems AddonUpdateChecker.jsm bypasses the cache when requesting the update
> manifest (using LOAD_BYPASS_CACHE), but doesn't inhibit writing to it. It
> should!
> 
> It sets LOAD_BYPASS_CACHE here:
> http://hg.mozilla.org/mozilla-central/file/1c0e964bd620/toolkit/mozapps/
> extensions/AddonUpdateChecker.jsm#l414
> 
> It should also set INHIBIT_CACHING, like what is done in bug 866564 in
> LightweightThemeManager.jsm:
> https://bugzilla.mozilla.org/page.cgi?id=splinter.
> html&bug=866564&attachment=742892
Comment 2 Blair McBride [:Unfocused] (UNAVAILABLE) 2013-04-30 16:05:16 PDT
Yes, you just need to set that flag - a one line fix, like what the patch in bug 866564 is doing.
Comment 3 Ritesh 2013-04-30 23:49:29 PDT
Created attachment 744048 [details] [diff] [review]
Just added flag for INHIBIT_CACHING
Comment 4 Blair McBride [:Unfocused] (UNAVAILABLE) 2013-05-02 21:18:53 PDT
Comment on attachment 744048 [details] [diff] [review]
Just added flag for INHIBIT_CACHING

Review of attachment 744048 [details] [diff] [review]:
-----------------------------------------------------------------

Great - thanks Ritesh :)

One small thing that I'll fix up for you: Be careful with spaces versus tabs. We use 2 spaces to indent lines in Mozilla code, not tabs (I personally prefer tabs, but its best to be consistent with the rest of the code).
Comment 5 Blair McBride [:Unfocused] (UNAVAILABLE) 2013-05-02 21:26:20 PDT
Landed on the fx-team branch, which will get merged into mozilla-central within a day or two:
https://hg.mozilla.org/integration/fx-team/rev/e8c1bde62025
Comment 6 Tim Taubert [:ttaubert] 2013-05-06 04:32:09 PDT
https://hg.mozilla.org/mozilla-central/rev/e8c1bde62025

Note You need to log in before you can comment on or make changes to this bug.