Closed Bug 463961 Opened 16 years ago Closed 15 years ago

Unnecessary fetch of all etags for cached calendars

Categories

(Calendar :: Provider: CalDAV, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: lmarcotte, Assigned: lmarcotte)

Details

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.3) Gecko/2008092414 Firefox/3.0.3
Build Identifier: Lightning 0.9

In calDavCalendar.js, we have the following check in caldav_safeRefresh() :

        if (!this.mCtag || !this.mFirstRefreshDone)

This is wrong since we'll get all etags for cached calendars upon startup for no good reasons. We should rather have this check :

	if (!this.mCtag || (!this.isCached && !this.mFirstRefreshDone))

Moreover, we should add a check for ctag matches in checkDavResourceType_oSC. So:

            if (ctag.length) {
                thisCalendar.mCtag = ctag;
                thisCalendar.mTargetCalendar.setMetaData("ctag", ctag);
                if (thisCalendar.verboseLogging()) {
                    LOG("CalDAV: initial ctag " + ctag + " for calendar " + thisCalendar.name);
                }
            }

should become :

            if (ctag.length) {
	        // We compare the stored ctag with the one we just got, if
	        // they don't match, we update the items.
	        if (ctag != thisCalendar.mCtag) {
		  var refreshEvent = thisCalendar.prepRefresh();
		  thisCalendar.getUpdatedItems(refreshEvent, aChangeLogListener);
	        }
                thisCalendar.mCtag = ctag;
                thisCalendar.mTargetCalendar.setMetaData("ctag", ctag);
                if (thisCalendar.verboseLogging()) {
                    dump("CalDAV: initial ctag " + ctag + " for calendar " + thisCalendar.name);
                }
            }





Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Needs investigation. Ludovic, would you be so kind and propose a trunk patch, too?
In fact, the code in caldav_safeRefresh() should stay as is but in checkDavResourceType_oSC, we should check if the ctag matches thisCalendar.mCtag and set thisCalendar.mFirstRefreshDone to true - something like:

            if (ctag.length) {
	      if (ctag == thisCalendar.mCtag) {
		thisCalendar.mFirstRefreshDone = true;
	      }
	      thisCalendar.mCtag = ctag;
	      thisCalendar.mTargetCalendar.setMetaData("ctag", ctag);
	      if (thisCalendar.verboseLogging()) {
		dump("CalDAV: initial ctag " + ctag + " for calendar " + thisCalendar.name);
	      }
            }
Whiteboard: [needs patch for code in comment#2]
Attached patch Proposed fixSplinter Review
Attachment #363574 - Flags: review?(philipp)
Comment on attachment 363574 [details] [diff] [review]
Proposed fix

Looks good, r=philipp
Attachment #363574 - Flags: review?(philipp) → review+
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/483c01abce31>

-> FIXED
Status: UNCONFIRMED → RESOLVED
Closed: 15 years ago
Keywords: qawanted
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [needs patch for code in comment#2]
Target Milestone: --- → 1.0
Assignee: nobody → lmarcotte
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.