Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: lib/downloader.js

Issue 11153017: Improve Synchronizer functionality (Closed)
Left Patch Set: Created July 17, 2013, 12:32 p.m.
Right Patch Set: Fixed nits Created Nov. 5, 2013, 6:39 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <http://adblockplus.org/>, 2 * This file is part of Adblock Plus <http://adblockplus.org/>,
3 * Copyright (C) 2006-2013 Eyeo GmbH 3 * Copyright (C) 2006-2013 Eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 50 matching lines...) Expand 10 before | Expand all | Expand 10 after
61 * Function that will yield downloadable objects on each check. 61 * Function that will yield downloadable objects on each check.
62 * @type Function 62 * @type Function
63 */ 63 */
64 dataSource: null, 64 dataSource: null,
65 65
66 /** 66 /**
67 * Maximal time interval that the checks can be left out until the soft 67 * Maximal time interval that the checks can be left out until the soft
68 * expiration interval increases. 68 * expiration interval increases.
69 * @type Integer 69 * @type Integer
70 */ 70 */
71 maxAbsenseInterval: 1 * MILLIS_IN_DAY, 71 maxAbsenceInterval: 1 * MILLIS_IN_DAY,
Thomas Greiner 2013/07/25 15:21:37 typo: maxAbsenceInterval
72 72
73 /** 73 /**
74 * Minimal time interval before retrying a download after an error. 74 * Minimal time interval before retrying a download after an error.
75 * @type Integer 75 * @type Integer
76 */ 76 */
77 minRetryInterval: 1 * MILLIS_IN_DAY, 77 minRetryInterval: 1 * MILLIS_IN_DAY,
78 78
79 /** 79 /**
80 * Maximal allowed expiration interval, larger expiration intervals will be 80 * Maximal allowed expiration interval, larger expiration intervals will be
81 * corrected. 81 * corrected.
(...skipping 33 matching lines...) Expand 10 before | Expand all | Expand 10 after
115 onDownloadError: null, 115 onDownloadError: null,
116 116
117 /** 117 /**
118 * Checks whether anything needs downloading. 118 * Checks whether anything needs downloading.
119 */ 119 */
120 _doCheck: function() 120 _doCheck: function()
121 { 121 {
122 let now = Date.now(); 122 let now = Date.now();
123 for each (let downloadable in this.dataSource()) 123 for each (let downloadable in this.dataSource())
124 { 124 {
125 if (downloadable.lastCheck && now - downloadable.lastCheck > this.maxAbsen seInterval) 125 if (downloadable.lastCheck && now - downloadable.lastCheck > this.maxAbsen ceInterval)
126 { 126 {
127 // No checks for a long time interval - user must have been offline, e.g . 127 // No checks for a long time interval - user must have been offline, e.g .
128 // during a weekend. Increase soft expiration to prevent load peaks on t he 128 // during a weekend. Increase soft expiration to prevent load peaks on t he
129 // server. 129 // server.
130 downloadable.softExpiration += now - downloadable.lastCheck; 130 downloadable.softExpiration += now - downloadable.lastCheck;
131 } 131 }
132 downloadable.lastCheck = now; 132 downloadable.lastCheck = now;
133 133
134 // Sanity check: do expiration times make sense? Make sure people changing 134 // Sanity check: do expiration times make sense? Make sure people changing
135 // system clock don't get stuck with outdated subscriptions. 135 // system clock don't get stuck with outdated subscriptions.
(...skipping 28 matching lines...) Expand all
164 164
165 /** 165 /**
166 * Checks whether an address is currently being downloaded. 166 * Checks whether an address is currently being downloaded.
167 */ 167 */
168 isDownloading: function(/**String*/ url) /**Boolean*/ 168 isDownloading: function(/**String*/ url) /**Boolean*/
169 { 169 {
170 return url in this._downloading; 170 return url in this._downloading;
171 }, 171 },
172 172
173 /** 173 /**
174 * Starts a download. 174 * Starts downloading for an object.
175 * @param {Downloadable} url the object to be downloaded 175 */
Felix Dahlke 2013/07/25 13:16:02 The doc strings are a bit inconsistent. The one ab
Wladimir Palant 2013/11/05 06:40:42 I generally use @param only when there is a need t
176 */ 176 download: function(/**Downloadable*/ downloadable)
177 download: function(downloadable)
178 { 177 {
179 // Make sure to detach download from the current execution context 178 // Make sure to detach download from the current execution context
180 Utils.runAsync(this._download.bind(this, downloadable, 0)); 179 Utils.runAsync(this._download.bind(this, downloadable, 0));
181 }, 180 },
182 181
183 /** 182 /**
184 * Generates the real download URL for an object by appending various 183 * Generates the real download URL for an object by appending various
185 * parameters. 184 * parameters.
186 */ 185 */
187 getDownloadUrl: function(/**Downloadable*/ downloadable) /** String*/ 186 getDownloadUrl: function(/**Downloadable*/ downloadable) /** String*/
188 { 187 {
189 let {addonName, addonVersion, application} = require("info"); 188 let {addonName, addonVersion, application, applicationVersion, platform, pla tformVersion} = require("info");
190 let url = downloadable.redirectURL || downloadable.url; 189 let url = downloadable.redirectURL || downloadable.url;
191 if (url.indexOf("?") >= 0) 190 if (url.indexOf("?") >= 0)
192 url += "&"; 191 url += "&";
193 else 192 else
194 url += "?"; 193 url += "?";
195 url += "addonName=" + encodeURIComponent(addonName) + 194 url += "addonName=" + encodeURIComponent(addonName) +
196 "&addonVersion=" + encodeURIComponent(addonVersion) + 195 "&addonVersion=" + encodeURIComponent(addonVersion) +
197 "&application=" + encodeURIComponent(application) + 196 "&application=" + encodeURIComponent(application) +
197 "&applicationVersion=" + encodeURIComponent(applicationVersion) +
198 "&platform=" + encodeURIComponent(platform) +
199 "&platformVersion=" + encodeURIComponent(platformVersion) +
198 "&lastVersion=" + encodeURIComponent(downloadable.lastVersion); 200 "&lastVersion=" + encodeURIComponent(downloadable.lastVersion);
199 return url; 201 return url;
200 }, 202 },
201 203
202 _download: function(downloadable, redirects) 204 _download: function(downloadable, redirects)
203 { 205 {
204 if (downloadable.url in this._downloading) 206 if (this.isDownloading(downloadable.url))
Thomas Greiner 2013/07/25 15:21:37 You could use this.isDownloading(downloadable.url)
Wladimir Palant 2013/11/05 06:40:42 It was hard to convince myself that microoptimizin
205 return; 207 return;
206 208
207 let downloadURL = this.getDownloadUrl(downloadable); 209 let downloadUrl = this.getDownloadUrl(downloadable);
Felix Dahlke 2013/07/25 13:16:02 Inconsistent camel casing :P
208 let request = null; 210 let request = null;
209 211
210 let errorCallback = function errorCallback(error) 212 let errorCallback = function errorCallback(error)
211 { 213 {
212 let channelStatus = -1; 214 let channelStatus = -1;
213 try 215 try
214 { 216 {
215 channelStatus = request.channel.status; 217 channelStatus = request.channel.status;
216 } catch (e) {} 218 } catch (e) {}
217 219
218 let responseStatus = ""; 220 let responseStatus = request.status;
219 try
220 {
221 responseStatus = request.channel.QueryInterface(Ci.nsIHttpChannel).respo nseStatus;
222 } catch (e) {}
223 221
224 Cu.reportError("Adblock Plus: Downloading URL " + downloadable.url + " fai led (" + error + ")\n" + 222 Cu.reportError("Adblock Plus: Downloading URL " + downloadable.url + " fai led (" + error + ")\n" +
225 "Download address: " + downloadURL + "\n" + 223 "Download address: " + downloadUrl + "\n" +
226 "Channel status: " + channelStatus + "\n" + 224 "Channel status: " + channelStatus + "\n" +
227 "Server response: " + responseStatus); 225 "Server response: " + responseStatus);
228 226
229 if (this.onDownloadError) 227 if (this.onDownloadError)
230 { 228 {
231 // Allow one extra redirect if the error handler gives us a redirect URL 229 // Allow one extra redirect if the error handler gives us a redirect URL
232 let redirectCallback = null; 230 let redirectCallback = null;
233 if (redirects <= this.maxRedirects) 231 if (redirects <= this.maxRedirects)
234 { 232 {
235 redirectCallback = function redirectCallback(url) 233 redirectCallback = function redirectCallback(url)
236 { 234 {
237 downloadable.redirectURL = url; 235 downloadable.redirectURL = url;
238 this._download(downloadable, redirects + 1); 236 this._download(downloadable, redirects + 1);
239 }.bind(this); 237 }.bind(this);
240 } 238 }
241 239
242 this.onDownloadError(downloadable, downloadURL, error, channelStatus, re sponseStatus, redirectCallback); 240 this.onDownloadError(downloadable, downloadUrl, error, channelStatus, re sponseStatus, redirectCallback);
243 } 241 }
244 }.bind(this); 242 }.bind(this);
245 243
246 try 244 try
247 { 245 {
248 request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].createInstance(Ci. nsIXMLHttpRequest); 246 request = new XMLHttpRequest();
249 request.mozBackgroundRequest = true; 247 request.mozBackgroundRequest = true;
250 request.open("GET", downloadURL); 248 request.open("GET", downloadUrl);
251 } 249 }
252 catch (e) 250 catch (e)
253 { 251 {
254 errorCallback("synchronize_invalid_url"); 252 errorCallback("synchronize_invalid_url");
255 return; 253 return;
256 } 254 }
257 255
258 try { 256 try {
259 request.overrideMimeType("text/plain"); 257 request.overrideMimeType("text/plain");
260 request.channel.loadFlags = request.channel.loadFlags | 258 request.channel.loadFlags = request.channel.loadFlags |
(...skipping 37 matching lines...) Expand 10 before | Expand all | Expand 10 after
298 if (redirects >= this.maxRedirects) 296 if (redirects >= this.maxRedirects)
299 errorCallback("synchronize_connection_error"); 297 errorCallback("synchronize_connection_error");
300 else 298 else
301 { 299 {
302 downloadable.redirectURL = url; 300 downloadable.redirectURL = url;
303 this._download(downloadable, redirects + 1); 301 this._download(downloadable, redirects + 1);
304 } 302 }
305 }.bind(this)); 303 }.bind(this));
306 }.bind(this), false); 304 }.bind(this), false);
307 305
306 request.send(null);
307
308 this._downloading[downloadable.url] = true; 308 this._downloading[downloadable.url] = true;
309 if (this.onDownloadStarted) 309 if (this.onDownloadStarted)
310 this.onDownloadStarted(downloadable); 310 this.onDownloadStarted(downloadable);
Thomas Greiner 2013/07/25 15:21:37 Seems weird to me that onDownloadStarted is fired
Wladimir Palant 2013/11/05 06:40:42 Well, nothing really wrong with it (assuming that
311 request.send(null);
312 }, 311 },
313 312
314 /** 313 /**
315 * Produces a soft and a hard expiration interval for a given supplied 314 * Produces a soft and a hard expiration interval for a given supplied
316 * expiration interval. 315 * expiration interval.
317 * @return {Array} soft and hard expiration interval 316 * @return {Array} soft and hard expiration interval
318 */ 317 */
319 processExpirationInterval: function(/**Integer*/ interval) 318 processExpirationInterval: function(/**Integer*/ interval)
320 { 319 {
321 interval = Math.min(Math.max(interval, 0), this.maxExpirationInterval); 320 interval = Math.min(Math.max(interval, 0), this.maxExpirationInterval);
(...skipping 51 matching lines...) Expand 10 before | Expand all | Expand 10 after
373 * @type Integer 372 * @type Integer
374 */ 373 */
375 softExpiration: 0, 374 softExpiration: 0,
376 375
377 /** 376 /**
378 * Hard expiration interval, this is fixed. 377 * Hard expiration interval, this is fixed.
379 * @type Integer 378 * @type Integer
380 */ 379 */
381 hardExpiration: 0, 380 hardExpiration: 0,
382 }; 381 };
LEFTRIGHT
« no previous file | no next file » | Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Toggle Comments ('s')

Powered by Google App Engine
This is Rietveld