Bug 906615 - Background thumbnail service shouldn't overwrite existing thumbnails, and foreground service shouldn't capture when thumbnails are fresh. r=markh

This commit is contained in:
Drew Willcoxon 2013-09-12 09:25:57 -07:00
Родитель 994cd04059
Коммит 8933291a63
8 изменённых файлов: 160 добавлений и 122 удалений

Просмотреть файл

@ -102,7 +102,7 @@ let gBrowserThumbnails = {
_capture: function Thumbnails_capture(aBrowser) {
if (this._shouldCapture(aBrowser))
PageThumbs.captureAndStore(aBrowser);
PageThumbs.captureAndStoreIfStale(aBrowser);
},
_delayedCapture: function Thumbnails_delayedCapture(aBrowser) {

Просмотреть файл

@ -134,7 +134,7 @@ Site.prototype = {
#ifndef RELEASE_BUILD
// request a staleness check for the thumbnail, which will cause page.js
// to be notified and call our refreshThumbnail() method.
BackgroundPageThumbs.captureIfStale(this.url);
BackgroundPageThumbs.captureIfMissing(this.url);
// but still display whatever thumbnail might be available now.
#endif
this.refreshThumbnail();

Просмотреть файл

@ -17,11 +17,6 @@ const DEFAULT_CAPTURE_TIMEOUT = 30000; // ms
const DESTROY_BROWSER_TIMEOUT = 60000; // ms
const FRAME_SCRIPT_URL = "chrome://global/content/backgroundPageThumbsContent.js";
// If a request for a thumbnail comes in and we find one that is "stale"
// (or don't find one at all) we automatically queue a request to generate a
// new one.
const MAX_THUMBNAIL_AGE_SECS = 172800; // 2 days == 60*60*24*2 == 172800 secs.
const TELEMETRY_HISTOGRAM_ID_PREFIX = "FX_THUMBNAILS_BG_";
// possible FX_THUMBNAILS_BG_CAPTURE_DONE_REASON telemetry values
@ -63,7 +58,7 @@ const BackgroundPageThumbs = {
capture: function (url, options={}) {
if (!PageThumbs._prefEnabled()) {
if (options.onDone)
schedule(() => options.onDone(null));
schedule(() => options.onDone(url));
return;
}
this._captureQueue = this._captureQueue || [];
@ -87,11 +82,8 @@ const BackgroundPageThumbs = {
},
/**
* Checks if an existing thumbnail for the specified URL is either missing
* or stale, and if so, queues a background request to capture it. That
* capture process will send a notification via the observer service on
* capture, so consumers should watch for such observations if they want to
* be notified of an updated thumbnail.
* Asynchronously captures a thumbnail of the given URL if one does not
* already exist. Otherwise does nothing.
*
* WARNING: BackgroundPageThumbs.jsm is currently excluded from release
* builds. If you use it, you must also exclude your caller when
@ -102,19 +94,21 @@ const BackgroundPageThumbs = {
* @param options An optional object that configures the capture. See
* capture() for description.
*/
captureIfStale: function PageThumbs_captureIfStale(url, options={}) {
PageThumbsStorage.isFileRecentForURL(url, MAX_THUMBNAIL_AGE_SECS).then(
result => {
if (result.ok) {
if (options.onDone)
options.onDone(url);
return;
}
this.capture(url, options);
}, err => {
captureIfMissing: function (url, options={}) {
// The fileExistsForURL call is an optimization, potentially but unlikely
// incorrect, and no big deal when it is. After the capture is done, we
// atomically test whether the file exists before writing it.
PageThumbsStorage.fileExistsForURL(url).then(exists => {
if (exists.ok) {
if (options.onDone)
options.onDone(url);
});
return;
}
this.capture(url, options);
}, err => {
if (options.onDone)
options.onDone(url);
});
},
/**
@ -387,7 +381,7 @@ Capture.prototype = {
return;
}
PageThumbs._store(this.url, data.finalURL, data.imageData, data.wasErrorResponse)
PageThumbs._store(this.url, data.finalURL, data.imageData, true)
.then(done, done);
},
};

Просмотреть файл

@ -17,6 +17,11 @@ const LATEST_STORAGE_VERSION = 3;
const EXPIRATION_MIN_CHUNK_SIZE = 50;
const EXPIRATION_INTERVAL_SECS = 3600;
// If a request for a thumbnail comes in and we find one that is "stale"
// (or don't find one at all) we automatically queue a request to generate a
// new one.
const MAX_THUMBNAIL_AGE_SECS = 172800; // 2 days == 60*60*24*2 == 172800 secs.
/**
* Name of the directory in the profile that contains the thumbnails.
*/
@ -308,6 +313,33 @@ this.PageThumbs = {
}).bind(this));
},
/**
* Checks if an existing thumbnail for the specified URL is either missing
* or stale, and if so, captures and stores it. Once the thumbnail is stored,
* an observer service notification will be sent, so consumers should observe
* such notifications if they want to be notified of an updated thumbnail.
*
* @param aBrowser The content window of this browser will be captured.
* @param aCallback The function to be called when finished (optional).
*/
captureAndStoreIfStale: function PageThumbs_captureAndStoreIfStale(aBrowser, aCallback) {
let url = aBrowser.currentURI.spec;
PageThumbsStorage.isFileRecentForURL(url).then(recent => {
if (!recent.ok &&
// Careful, the call to PageThumbsStorage is async, so the browser may
// have navigated away from the URL or even closed.
aBrowser.currentURI &&
aBrowser.currentURI.spec == url) {
this.captureAndStore(aBrowser, aCallback);
} else if (aCallback) {
aCallback(true);
}
}, err => {
if (aCallback)
aCallback(false);
});
},
/**
* Stores data to disk for the given URLs.
*
@ -316,29 +348,14 @@ this.PageThumbs = {
* @param aOriginalURL The URL with which the capture was initiated.
* @param aFinalURL The URL to which aOriginalURL ultimately resolved.
* @param aData An ArrayBuffer containing the image data.
* @param aWasErrorResponse A boolean indicating if the capture was for a
* response that returned an error code.
* @param aNoOverwrite If true and files for the URLs already exist, the files
* will not be overwritten.
* @return {Promise}
*/
_store: function PageThumbs__store(aOriginalURL, aFinalURL, aData, aWasErrorResponse) {
_store: function PageThumbs__store(aOriginalURL, aFinalURL, aData, aNoOverwrite) {
return TaskUtils.spawn(function () {
// If we got an error response, we only save it if we don't have an
// existing thumbnail. If we *do* have an existing thumbnail we "touch"
// it so we consider the old version fresh.
if (aWasErrorResponse) {
let result = yield PageThumbsStorage.touchIfExists(aFinalURL);
let exists = result.ok;
if (exists) {
if (aFinalURL != aOriginalURL) {
yield PageThumbsStorage.touchIfExists(aOriginalURL);
}
return;
}
// was an error response, but no existing thumbnail - just store
// that error response as something is (arguably) better than nothing.
}
let telemetryStoreTime = new Date();
yield PageThumbsStorage.writeData(aFinalURL, aData);
yield PageThumbsStorage.writeData(aFinalURL, aData, aNoOverwrite);
Services.telemetry.getHistogramById("FX_THUMBNAILS_STORE_TIME_MS")
.add(new Date() - telemetryStoreTime);
@ -355,7 +372,7 @@ this.PageThumbs = {
// Sync and therefore also redirect sources appear on the newtab
// page. We also want thumbnails for those.
if (aFinalURL != aOriginalURL) {
yield PageThumbsStorage.copy(aFinalURL, aOriginalURL);
yield PageThumbsStorage.copy(aFinalURL, aOriginalURL, aNoOverwrite);
Services.obs.notifyObservers(null, "page-thumbnail:create", aOriginalURL);
}
});
@ -530,10 +547,12 @@ this.PageThumbsStorage = {
* @param {ArrayBuffer} aData The data to store in the thumbnail, as
* an ArrayBuffer. This array buffer is neutered and cannot be
* reused after the copy.
* @param {boolean} aNoOverwrite If true and the thumbnail's file already
* exists, the file will not be overwritten.
*
* @return {Promise}
*/
writeData: function Storage_writeData(aURL, aData) {
writeData: function Storage_writeData(aURL, aData, aNoOverwrite) {
let path = this.getFilePathForURL(aURL);
this.ensurePath();
aData = new Uint8Array(aData);
@ -543,12 +562,14 @@ this.PageThumbsStorage = {
{
tmpPath: path + ".tmp",
bytes: aData.byteLength,
noOverwrite: aNoOverwrite,
flush: false /*thumbnails do not require the level of guarantee provided by flush*/
}];
return PageThumbsWorker.post("writeAtomic", msg,
msg /*we don't want that message garbage-collected,
as OS.Shared.Type.void_t.in_ptr.toMsg uses C-level
memory tricks to enforce zero-copy*/);
memory tricks to enforce zero-copy*/).
then(null, this._eatNoOverwriteError(aNoOverwrite));
},
/**
@ -556,14 +577,18 @@ this.PageThumbsStorage = {
*
* @param {string} aSourceURL The url of the thumbnail to copy.
* @param {string} aTargetURL The url of the target thumbnail.
* @param {boolean} aNoOverwrite If true and the target file already exists,
* the file will not be overwritten.
*
* @return {Promise}
*/
copy: function Storage_copy(aSourceURL, aTargetURL) {
copy: function Storage_copy(aSourceURL, aTargetURL, aNoOverwrite) {
this.ensurePath();
let sourceFile = this.getFilePathForURL(aSourceURL);
let targetFile = this.getFilePathForURL(aTargetURL);
return PageThumbsWorker.post("copy", [sourceFile, targetFile]);
let options = { noOverwrite: aNoOverwrite };
return PageThumbsWorker.post("copy", [sourceFile, targetFile, options]).
then(null, this._eatNoOverwriteError(aNoOverwrite));
},
/**
@ -584,20 +609,14 @@ this.PageThumbsStorage = {
return PageThumbsWorker.post("wipe", [this.path]);
},
/**
* If the file holding the thumbnail for the given URL exists, update the
* modification time of the file to now and return true, otherwise return
* false.
*
* @return {Promise}
*/
touchIfExists: function Storage_touchIfExists(aURL) {
return PageThumbsWorker.post("touchIfExists", [this.getFilePathForURL(aURL)]);
fileExistsForURL: function Storage_fileExistsForURL(aURL) {
return PageThumbsWorker.post("exists", [this.getFilePathForURL(aURL)]);
},
isFileRecentForURL: function Storage_isFileRecentForURL(aURL, aMaxSecs) {
isFileRecentForURL: function Storage_isFileRecentForURL(aURL) {
return PageThumbsWorker.post("isFileRecent",
[this.getFilePathForURL(aURL), aMaxSecs]);
[this.getFilePathForURL(aURL),
MAX_THUMBNAIL_AGE_SECS]);
},
_calculateMD5Hash: function Storage_calculateMD5Hash(aValue) {
@ -616,6 +635,26 @@ this.PageThumbsStorage = {
return hex;
},
/**
* For functions that take a noOverwrite option, OS.File throws an error if
* the target file exists and noOverwrite is true. We don't consider that an
* error, and we don't want such errors propagated.
*
* @param {aNoOverwrite} The noOverwrite option used in the OS.File operation.
*
* @return {function} A function that should be passed as the second argument
* to then() (the `onError` argument).
*/
_eatNoOverwriteError: function Storage__eatNoOverwriteError(aNoOverwrite) {
return function onError(err) {
if (!aNoOverwrite ||
!(err instanceof OS.File.Error) ||
!err.becauseExists) {
throw err;
}
};
},
// Deprecated, please do not use
getFileForURL: function Storage_getFileForURL_DEPRECATED(aURL) {
Deprecated.warning("PageThumbs.getFileForURL is deprecated. Please use PageThumbs.getFilePathForURL and OS.File",

Просмотреть файл

@ -153,8 +153,8 @@ let Agent = {
return File.makeDir(path, options);
},
copy: function Agent_copy(source, dest) {
return File.copy(source, dest);
copy: function Agent_copy(source, dest, options) {
return File.copy(source, dest, options);
},
wipe: function Agent_wipe(path) {
@ -178,26 +178,8 @@ let Agent = {
}
},
touchIfExists: function Agent_touchIfExists(path) {
// No OS.File way to update the modification date of the file (bug 905509)
// so we open it for reading and writing, read 1 byte from the start of
// the file then write that byte back out.
// (Sadly it's impossible to use nsIFile here as we have no access to
// |Components|)
if (!File.exists(path)) {
return false;
}
let file = OS.File.open(path, { read: true, write: true });
try {
file.setPosition(0); // docs aren't clear on initial position, so seek to 0.
let byte = file.read(1);
file.setPosition(0);
file.write(byte);
} finally {
file.close();
}
return true;
exists: function Agent_exists(path) {
return File.exists(path);
},
};

Просмотреть файл

@ -72,8 +72,6 @@ const backgroundPageThumbsContent = {
PageThumbs._captureToCanvas(content, canvas);
let captureTime = new Date() - captureDate;
let channel = docShell.currentDocumentChannel;
let isErrorResponse = PageThumbs._isChannelErrorResponse(channel);
let finalURL = this._webNav.currentURI.spec;
let fileReader = Cc["@mozilla.org/files/filereader;1"].
createInstance(Ci.nsIDOMFileReader);
@ -82,7 +80,6 @@ const backgroundPageThumbsContent = {
id: requestID,
imageData: fileReader.result,
finalURL: finalURL,
wasErrorResponse: isErrorResponse,
telemetry: {
CAPTURE_PAGE_LOAD_TIME_MS: pageLoadTime,
CAPTURE_CANVAS_DRAW_TIME_MS: captureTime,

Просмотреть файл

@ -328,15 +328,45 @@ let tests = [
imports.BackgroundPageThumbs.capture(url, {onDone: doneCallback});
yield deferred.promise;
},
function capIfMissing() {
let url = "http://example.com/";
let file = fileForURL(url);
ok(!file.exists(), "Thumbnail file should not already exist.");
let capturedURL = yield captureIfMissing(url);
is(capturedURL, url, "Captured URL should be URL passed to capture");
ok(file.exists(), "Thumbnail should be cached after capture: " + file.path);
let past = Date.now() - 1000000000;
let pastFudge = past + 30000;
file.lastModifiedTime = past;
ok(file.lastModifiedTime < pastFudge, "Last modified time should stick!");
capturedURL = yield captureIfMissing(url);
is(capturedURL, url, "Captured URL should be URL passed to second capture");
ok(file.exists(), "Thumbnail should remain cached after second capture: " +
file.path);
ok(file.lastModifiedTime < pastFudge,
"File should not have been overwritten");
file.remove(false);
},
];
function capture(url, options) {
return captureWithMethod("capture", url, options);
}
function captureIfMissing(url, options) {
return captureWithMethod("captureIfMissing", url, options);
}
function captureWithMethod(methodName, url, options={}) {
let deferred = imports.Promise.defer();
options = options || {};
options.onDone = function onDone(capturedURL) {
deferred.resolve(capturedURL);
};
imports.BackgroundPageThumbs.capture(url, options);
imports.BackgroundPageThumbs[methodName](url, options);
return deferred.promise;
}

Просмотреть файл

@ -2,24 +2,17 @@
http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* These tests check the auto-update facility of the background thumbnail
* service.
* These tests check the auto-update facility of the thumbnail service.
*/
const imports = {};
Cu.import("resource://gre/modules/BackgroundPageThumbs.jsm", imports);
registerCleanupFunction(function () {
imports.BackgroundPageThumbs._destroy();
});
function runTests() {
// A "trampoline" - a generator that iterates over sub-iterators
let tests = [
simpleCaptureTest,
errorResponseUpdateTest,
goodResponseUpdateTest,
foregroundErrorResponseUpdateTest,
foregroundGoodResponseUpdateTest
capIfStaleErrorResponseUpdateTest,
capIfStaleGoodResponseUpdateTest,
regularCapErrorResponseUpdateTest,
regularCapGoodResponseUpdateTest
];
for (let test of tests) {
info("Running subtest " + test.name);
@ -47,7 +40,7 @@ function getThumbnailModifiedTime(url) {
}
// The tests!
/* Check functionality of a normal "captureIfStale" request */
/* Check functionality of a normal captureAndStoreIfStale request */
function simpleCaptureTest() {
let numNotifications = 0;
const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?simple";
@ -58,6 +51,7 @@ function simpleCaptureTest() {
if (++numNotifications == 2) {
// This is the final notification and signals test success...
Services.obs.removeObserver(observe, "page-thumbnail:create");
gBrowser.removeTab(gBrowser.selectedTab);
next();
}
}
@ -69,58 +63,56 @@ function simpleCaptureTest() {
// Capture the screenshot.
PageThumbs.captureAndStore(browser, function () {
// done with the tab.
gBrowser.removeTab(gBrowser.selectedTab);
// We've got a capture so should have seen the observer.
is(numNotifications, 1, "got notification of item being created.");
// The capture is now "fresh" - so requesting the URL should not cause
// a new capture.
imports.BackgroundPageThumbs.captureIfStale(URL);
PageThumbs.captureAndStoreIfStale(browser);
is(numNotifications, 1, "still only 1 notification of item being created.");
ensureThumbnailStale(URL);
// Ask for it to be updated.
imports.BackgroundPageThumbs.captureIfStale(URL);
PageThumbs.captureAndStoreIfStale(browser);
// But it's async, so wait - our observer above will call next() when
// the notification comes.
});
yield undefined // wait for callbacks to call 'next'...
}
/* Check functionality of a background capture when there is an error response
/* Check functionality of captureAndStoreIfStale when there is an error response
from the server.
*/
function errorResponseUpdateTest() {
function capIfStaleErrorResponseUpdateTest() {
const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?fail";
yield addTab(URL);
yield captureAndCheckColor(0, 255, 0, "we have a green thumbnail");
gBrowser.removeTab(gBrowser.selectedTab);
// update the thumbnail to be stale, then re-request it. The server will
// return a 400 response and a red thumbnail.
// The b/g service should (a) not save the thumbnail and (b) update the file
// to have an mtime of "now" - so we (a) check the thumbnail remains green
// and (b) check the mtime of the file is >= now.
// The service should not save the thumbnail - so we (a) check the thumbnail
// remains green and (b) check the mtime of the file is < now.
ensureThumbnailStale(URL);
yield navigateTo(URL);
// now() returns a higher-precision value than the modified time of a file.
// As we set the thumbnail very stale, allowing 1 second of "slop" here
// works around this while still keeping the test valid.
let now = Date.now() - 1000 ;
imports.BackgroundPageThumbs.captureIfStale(URL, { onDone: () => {
ok(getThumbnailModifiedTime(URL) >= now, "modified time should be >= now");
PageThumbs.captureAndStoreIfStale(gBrowser.selectedBrowser, () => {
ok(getThumbnailModifiedTime(URL) < now, "modified time should be < now");
retrieveImageDataForURL(URL, function ([r, g, b]) {
is("" + [r,g,b], "" + [0, 255, 0], "thumbnail is still green");
gBrowser.removeTab(gBrowser.selectedTab);
next();
});
}});
});
yield undefined; // wait for callback to call 'next'...
}
/* Check functionality of a background capture when there is a non-error
/* Check functionality of captureAndStoreIfStale when there is a non-error
response from the server. This test is somewhat redundant - although it is
using a http:// URL instead of a data: url like most others.
*/
function goodResponseUpdateTest() {
function capIfStaleGoodResponseUpdateTest() {
const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?ok";
yield addTab(URL);
let browser = gBrowser.selectedBrowser;
@ -130,26 +122,27 @@ function goodResponseUpdateTest() {
// return a 200 response and a red thumbnail - so that new thumbnail should
// end up captured.
ensureThumbnailStale(URL);
yield navigateTo(URL);
// now() returns a higher-precision value than the modified time of a file.
// As we set the thumbnail very stale, allowing 1 second of "slop" here
// works around this while still keeping the test valid.
let now = Date.now() - 1000 ;
imports.BackgroundPageThumbs.captureIfStale(URL, { onDone: () => {
PageThumbs.captureAndStoreIfStale(browser, () => {
ok(getThumbnailModifiedTime(URL) >= now, "modified time should be >= now");
// the captureIfStale request saw a 200 response with the red body, so we
// expect to see the red version here.
// the captureAndStoreIfStale request saw a 200 response with the red body,
// so we expect to see the red version here.
retrieveImageDataForURL(URL, function ([r, g, b]) {
is("" + [r,g,b], "" + [255, 0, 0], "thumbnail is now red");
next();
});
}});
});
yield undefined; // wait for callback to call 'next'...
}
/* Check functionality of a foreground capture when there is an error response
/* Check functionality of captureAndStore when there is an error response
from the server.
*/
function foregroundErrorResponseUpdateTest() {
function regularCapErrorResponseUpdateTest() {
const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?fail";
yield addTab(URL);
yield captureAndCheckColor(0, 255, 0, "we have a green thumbnail");
@ -160,7 +153,10 @@ function foregroundErrorResponseUpdateTest() {
yield captureAndCheckColor(0, 255, 0, "we still have a green thumbnail");
}
function foregroundGoodResponseUpdateTest() {
/* Check functionality of captureAndStore when there is an OK response
from the server.
*/
function regularCapGoodResponseUpdateTest() {
const URL = "http://mochi.test:8888/browser/toolkit/components/thumbnails/test/thumbnails_update.sjs?ok";
yield addTab(URL);
yield captureAndCheckColor(0, 255, 0, "we have a green thumbnail");