Bug 938829 - Optimize attachmentWorker.onmessage to not construct the whole attachment notification bar when it is already shown (from Suyash Agarwal) and rework the state machine deciding when to show notification (from aceman). r=mkmelin

This commit is contained in:
aceman 2014-06-20 15:22:00 -04:00
Родитель 07eebee281
Коммит f9317e9bda
2 изменённых файлов: 275 добавлений и 133 удалений

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

@ -1825,113 +1825,201 @@ function handleEsc()
}
}
/**
* This state machine manages all showing and hiding of the attachment
* notification bar. It is only called if any change happened so that
* recalculating of the notification is needed:
* - keywords changed
* - manual reminder was toggled
* - attachments changed
*
* It does not track whether the notification is still up when it should be.
* That allows the user to close it any time without this function showing
* it again.
* We ensure notification is only shown on right events, e.g. only when we have
* keywords and attachments were removed (but not when we have keywords and
* manual reminder was just turned off). We always show the notification
* again if keywords change (if no attachments and no manual reminder).
*
* @param aForce If set to true, notification will be shown immediately if
* there are any keywords. If set to true, it is shown only when
* they have changed.
*/
function manageAttachmentNotification(aForce)
{
let keywords;
let keywordsCount = 0;
// First see if the notification is to be hidden due to reasons other than
// not having keywords.
let removeNotification = attachmentNotificationSupressed();
// If that is not true, we need to look at the state of keywords.
if (!removeNotification) {
if (attachmentWorker.lastMessage) {
// We know the state of keywords, so process them.
if (attachmentWorker.lastMessage.length) {
keywords = attachmentWorker.lastMessage.join(", ");
keywordsCount = attachmentWorker.lastMessage.length;
}
removeNotification = keywordsCount == 0;
} else {
// We don't know keywords, so get them first.
// If aForce was true, and some keywords are found, we get to run again from
// attachmentWorker.onmessage().
redetectKeywords(aForce);
return;
}
}
let nBox = document.getElementById("attachmentNotificationBox");
let notification = nBox.getNotificationWithValue("attachmentReminder");
if (removeNotification) {
if (notification)
nBox.removeNotification(notification);
return;
}
// We have some keywords, however only pop up the notification if requested
// to do so.
if (!aForce)
return;
let textValue = getComposeBundle().getString("attachmentReminderKeywordsMsgs");
textValue = PluralForm.get(keywordsCount, textValue)
.replace("#1", keywordsCount);
// If the notification already exists, we simply add the new attachment
// specific keywords to the existing notification instead of creating it
// from scratch.
if (notification) {
let description = notification.querySelector("#attachmentReminderText");
description.setAttribute("value", textValue);
description = notification.querySelector("#attachmentKeywords");
description.setAttribute("value", keywords);
return;
}
// Construct the notification as we don't have one.
let msg = document.createElement("hbox");
msg.setAttribute("flex", "100");
msg.onclick = function(event) {
openOptionsDialog("paneCompose", "generalTab",
{subdialog: "attachment_reminder_button"});
};
let msgText = document.createElement("label");
msg.appendChild(msgText);
msgText.id = "attachmentReminderText";
msgText.setAttribute("crop", "end");
msgText.setAttribute("flex", "1");
msgText.setAttribute("value", textValue);
let msgKeywords = document.createElement("label");
msg.appendChild(msgKeywords);
msgKeywords.id = "attachmentKeywords";
msgKeywords.setAttribute("crop", "end");
msgKeywords.setAttribute("flex", "1000");
msgKeywords.setAttribute("value", keywords);
let addButton = {
accessKey : getComposeBundle().getString("addAttachmentButton.accesskey"),
label: getComposeBundle().getString("addAttachmentButton"),
callback: function (aNotificationBar, aButton) {
goDoCommand("cmd_attachFile");
}
};
let remindButton = {
accessKey : getComposeBundle().getString("remindLaterButton.accesskey"),
label: getComposeBundle().getString("remindLaterButton"),
callback: function (aNotificationBar, aButton) {
toggleAttachmentReminder(true);
}
};
notification = nBox.appendNotification("", "attachmentReminder",
/* fake out the image so we can do it in CSS */
"null",
nBox.PRIORITY_WARNING_MEDIUM,
[addButton, remindButton]);
let buttons = notification.childNodes[0];
notification.insertBefore(msg, buttons);
}
/**
* Returns whether the attachment notification should be supressed regardless of
* the state of keywords.
*/
function attachmentNotificationSupressed() {
return (gManualAttachmentReminder ||
document.getElementById("attachmentBucket").itemCount);
}
var attachmentWorker = new Worker("resource:///modules/attachmentChecker.js");
// The array of currently found keywords. Or null if keyword detection wasn't
// run yet so we don't know.
attachmentWorker.lastMessage = null;
attachmentWorker.onerror = function(error)
{
dump("Attachment Notification Worker error!!! " + error.message + "\n");
Components.utils.reportError("Attachment Notification Worker error!!! " + error.message);
throw error;
};
attachmentWorker.onmessage = function(event)
/**
* Called when attachmentWorker finishes checking of the message for keywords.
*
* @param event If defined, event.data contains an array of found keywords.
* @param aManage If set to true and we determine keywords have changed,
* manage the notification.
* If set to false, just store the new keyword list but do not
* touch the notification. That effectively eats the
* "keywords changed" event which usually shows the notification
* if it was hidden. See manageAttachmentNotification().
*/
attachmentWorker.onmessage = function(event, aManage = true)
{
let keywordsFound = event.data;
let msg = null;
let nBox = document.getElementById("attachmentNotificationBox");
let notification = nBox.getNotificationWithValue("attachmentReminder");
let removeNotification = false;
// Exit if keywords haven't changed.
if (!event || (attachmentWorker.lastMessage &&
(event.data.toString() == attachmentWorker.lastMessage.toString())))
return;
if (keywordsFound.length > 0) {
msg = document.createElement("hbox");
msg.setAttribute("flex", "100");
msg.onclick = function(event)
{
openOptionsDialog("paneCompose", "generalTab",
{subdialog: "attachment_reminder_button"});
};
let msgText = document.createElement("label");
msg.appendChild(msgText);
msgText.id = "attachmentReminderText";
msgText.setAttribute("crop", "end");
msgText.setAttribute("flex", "1");
let textValue = getComposeBundle().getString("attachmentReminderKeywordsMsgs");
textValue = PluralForm.get(keywordsFound.length, textValue)
.replace("#1", keywordsFound.length);
msgText.setAttribute("value", textValue);
let keywords = keywordsFound.join(", ");
let msgKeywords = document.createElement("label");
msg.appendChild(msgKeywords);
msgKeywords.id = "attachmentKeywords";
msgKeywords.setAttribute("crop", "end");
msgKeywords.setAttribute("flex", "1000");
msgKeywords.setAttribute("value", keywords);
if (notification) {
let description = notification.querySelector("#attachmentReminderText");
description.setAttribute("value", msgText.getAttribute("value"));
description = notification.querySelector("#attachmentKeywords")
description.setAttribute("value", keywords);
msg = null;
}
if (keywords == this.lastMessage) {
// The user closed the notification, and we have nothing new to say.
msg = null;
}
this.lastMessage = keywords;
}
else {
removeNotification = true;
this.lastMessage = null;
}
if (notification && removeNotification)
nBox.removeNotification(notification);
if (msg) {
var addButton = {
accessKey : getComposeBundle().getString("addAttachmentButton.accesskey"),
label: getComposeBundle().getString("addAttachmentButton"),
callback: function (aNotificationBar, aButton)
{
goDoCommand("cmd_attachFile");
}
};
var remindButton = {
accessKey : getComposeBundle().getString("remindLaterButton.accesskey"),
label: getComposeBundle().getString("remindLaterButton"),
callback: function (aNotificationBar, aButton)
{
toggleAttachmentReminder(true);
}
};
notification = nBox.appendNotification("", "attachmentReminder",
/* fake out the image so we can do it in CSS */
"null",
nBox.PRIORITY_WARNING_MEDIUM,
[addButton, remindButton]);
let buttons = notification.childNodes[0];
notification.insertBefore(msg, buttons);
}
CheckForAttachmentNotification.shouldFire = true;
let data = event ? event.data : [];
attachmentWorker.lastMessage = data.slice(0);
if (aManage)
manageAttachmentNotification(true);
};
/**
* Determine whether we should show the attachment notification or not.
* Checks for new keywords synchronously and run the usual handler.
*
* @param async Whether we should run the regex checker asynchronously or not.
* @return true if we should show the attachment notification
* @param aManage Determines whether to manage the notification according to keywords found.
*/
function ShouldShowAttachmentNotification(async)
function redetectKeywords(aManage) {
attachmentWorker.onmessage({ data: CheckForAttachmentKeywords(false) }, aManage);
}
/**
* Check if there are any keywords in the message.
*
* @param async Whether we should run the regex checker asynchronously or not.
*
* @return If async is true, attachmentWorker.message is called with the array
* of found keywords and this function returns null.
* If it is false, the array is returned from this function immediately.
*/
function CheckForAttachmentKeywords(async)
{
let bucket = document.getElementById("attachmentBucket");
let warn = getPref("mail.compose.attachment_reminder");
if (warn && !bucket.itemCount) {
if (warn) {
if (attachmentNotificationSupressed()) {
// If we know we don't need to show the notification,
// we can skip the expensive checking of keywords in the message.
// but mark it in the .lastMessage that the keywords are unknown.
attachmentWorker.lastMessage = null;
return false;
}
let keywordsInCsv = Services.prefs.getComplexValue(
"mail.compose.attachment_reminder_keywords",
Components.interfaces.nsIPrefLocalizedString).data;
@ -1996,38 +2084,20 @@ function ShouldShowAttachmentNotification(async)
mailData = subject + " " + mailData;
if (!async)
return GetAttachmentKeywords(mailData, keywordsInCsv).length != 0;
return GetAttachmentKeywords(mailData, keywordsInCsv);
attachmentWorker.postMessage([mailData, keywordsInCsv]);
return true;
return null;
}
return false;
}
/**
* Check for attachment keywords, and display a notification if it's
* appropriate.
* Called when number of attachments changes.
*/
function CheckForAttachmentNotification(event)
{
if (!CheckForAttachmentNotification.shouldFire || gManualAttachmentReminder)
return;
if (!event)
attachmentWorker.lastMessage = null;
CheckForAttachmentNotification.shouldFire = false;
let nBox = document.getElementById("attachmentNotificationBox");
let notification = nBox.getNotificationWithValue("attachmentReminder");
let removeNotification = false;
if (!ShouldShowAttachmentNotification(true)) {
removeNotification = true;
CheckForAttachmentNotification.shouldFire = true;
}
if (notification && removeNotification)
nBox.removeNotification(notification);
};
CheckForAttachmentNotification.shouldFire = true;
function AttachmentsChanged() {
manageAttachmentNotification(true);
}
function ComposeStartup(recycled, aParams)
{
@ -2584,7 +2654,8 @@ function GenericSendMessage(msgType)
null, null, {value:0});
// Deactivate manual attachment reminder after showing the alert to avoid alert loop.
// We also deactivate reminder when user ignores alert with [x] or [ESC].
toggleAttachmentReminder(false);
if (gManualAttachmentReminder)
toggleAttachmentReminder(false);
if (hadForgotten)
return;
@ -3233,10 +3304,7 @@ function toggleAttachmentReminder(aState = !gManualAttachmentReminder)
document.getElementById("cmd_remindLater")
.setAttribute("checked", aState);
gMsgCompose.compFields.attachmentReminder = aState;
let nBox = document.getElementById("attachmentNotificationBox");
let notification = nBox.getNotificationWithValue("attachmentReminder");
if (aState && notification)
nBox.removeNotification(notification);
manageAttachmentNotification(false);
}
function ClearIdentityListPopup(popup)
@ -3626,7 +3694,7 @@ function AddAttachments(aAttachments, aCallback)
if (aCallback)
aCallback(item);
CheckForAttachmentNotification(null);
AttachmentsChanged();
}
if (addedAttachments.length) {
@ -3721,7 +3789,7 @@ function RemoveAllAttachments()
dispatchAttachmentBucketEvent("attachments-removed", removedAttachments);
UpdateAttachmentBucket(false);
CheckForAttachmentNotification(null);
AttachmentsChanged();
}
/**
@ -3778,7 +3846,7 @@ function RemoveSelectedAttachment()
gContentChanged = true;
dispatchAttachmentBucketEvent("attachments-removed", removedAttachments);
}
CheckForAttachmentNotification(null);
AttachmentsChanged();
}
function RenameSelectedAttachment()
@ -4587,8 +4655,11 @@ const gAttachmentNotifier =
{
// Only run the checker if the compose window is initialized
// and not shutting down.
if (gMsgCompose)
CheckForAttachmentNotification(true);
if (gMsgCompose) {
// This runs the attachmentWorker asynchronously so if keywords are found
// manageAttachmentNotification is run from attachmentWorker.onmessage.
CheckForAttachmentKeywords(true);
}
}
},

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

@ -50,9 +50,16 @@ function assert_automatic_reminder_state(aCwc, aShown) {
*
* @param aCwc A compose window controller.
* @param aShown True for waiting for the bar to be shown, false otherwise.
* @param aDelay Set to true to sleep a while to give the notification time
* to change. This is used if the state is already what we want
* but we expect it could change in a short while.
*/
function wait_for_reminder_state(aCwc, aShown) {
function wait_for_reminder_state(aCwc, aShown, aDelay = false) {
const notificationSlackTime = 5000;
if (aShown) {
if (aDelay)
aCwc.sleep(notificationSlackTime);
// This waits up to 30 seconds for the notification to appear.
wait_for_notification_to_show(aCwc, kBoxId, kNotificationId);
} else if (check_notification_displayed(aCwc, kBoxId, kNotificationId)) {
@ -60,7 +67,7 @@ function wait_for_reminder_state(aCwc, aShown) {
wait_for_notification_to_stop(aCwc, kBoxId, kNotificationId);
} else {
// This waits 5 seconds during which the notification must not appear.
aCwc.sleep(5000);
aCwc.sleep(notificationSlackTime);
assert_automatic_reminder_state(aCwc, false);
}
}
@ -83,12 +90,24 @@ function assert_manual_reminder_state(aCwc, aChecked) {
assert_equals(aCwc.e("cmd_remindLater").getAttribute("checked"), checkedValue);
}
/**
* Returns the keywords string currently shown in the notification message.
*
* @param aCwc A compose window controller.
*/
function get_reminder_keywords(aCwc) {
assert_automatic_reminder_state(aCwc, true);
let nBox = aCwc.e(kBoxId);
let notification = nBox.getNotificationWithValue(kNotificationId);
return notification.querySelector("#attachmentKeywords").getAttribute("value");
}
/**
* Test that the attachment reminder works, in general.
*/
function test_attachment_reminder_appears_properly() {
let cwc = open_compose_new_mail();
let notificationBox = cwc.e(kBoxId);
// There should be no notification yet.
assert_automatic_reminder_state(cwc, false);
@ -156,6 +175,48 @@ function test_attachment_reminder_dismissal() {
close_compose_window(cwc);
}
/**
* Bug 938829
* Check that adding an attachment actually hides the notification.
*/
function test_attachment_reminder_with_attachment() {
let cwc = open_compose_new_mail();
// There should be no notification yet.
assert_automatic_reminder_state(cwc, false);
setupComposeWin(cwc, "test@example.org", "Testing automatic reminder!",
"Hello! We will have a real attachment here.");
// Give the notification time to appear. It should.
wait_for_reminder_state(cwc, true);
// Add an attachment.
let file = Services.dirsvc.get("ProfD", Components.interfaces.nsIFile);
file.append("panacea.dat");
assert_true(file.exists(), "The required file panacea.dat was not found in the profile.");
let attachment = [cwc.window.FileToAttachment(file)];
cwc.window.AddAttachments(attachment);
// The notification should hide.
wait_for_reminder_state(cwc, false);
// Add some more text with keyword so the automatic notification
// could potentially show up.
setupComposeWin(cwc, "", "", " Yes, there is a file attached!");
// Give the notification time to appear. It shouldn't.
wait_for_reminder_state(cwc, false);
cwc.window.RemoveAllAttachments();
// After removing the attachment, notification should come back
// with all the keywords, even those input while having an attachment.
wait_for_reminder_state(cwc, true);
assert_equals(get_reminder_keywords(cwc), "attachment, attached");
close_compose_window(cwc);
}
/**
* Test that the mail.compose.attachment_reminder_aggressive pref works.
*/
@ -311,9 +372,9 @@ function test_manual_automatic_attachment_reminder_interaction() {
// The attachment notification should disappear.
wait_for_reminder_state(cwc, false);
// Add some more text with another keyword so the automatic notification
// Add some more text so the automatic notification
// could potentially show up.
setupComposeWin(cwc, "", "", " and find it attached!");
setupComposeWin(cwc, "", "", " and look for your attachment!");
// Give the notification time to appear. It shouldn't.
wait_for_reminder_state(cwc, false);
@ -322,10 +383,16 @@ function test_manual_automatic_attachment_reminder_interaction() {
// Give the notification time to appear. It shouldn't.
wait_for_reminder_state(cwc, false);
// Add some more text without any new keyword.
setupComposeWin(cwc, "", "", " Did I write anything?");
// Add some more text without keywords.
setupComposeWin(cwc, "", "", " No keywords here.");
// Give the notification time to appear. It shouldn't.
wait_for_reminder_state(cwc, false);
// Add some more text with a new keyword.
setupComposeWin(cwc, "", "", " Do you find it attached?");
// Give the notification time to appear. It should now.
wait_for_reminder_state(cwc, true);
assert_equals(get_reminder_keywords(cwc), "attachment, attached");
close_compose_window(cwc);
}
@ -390,6 +457,7 @@ function test_attachment_reminder_in_subject() {
// The automatic attachment notification should pop up.
wait_for_reminder_state(cwc, true);
assert_equals(get_reminder_keywords(cwc), "attachment");
// Now clear the subject
delete_all_existing(cwc, cwc.eid("msgSubject"));
@ -419,12 +487,15 @@ function test_attachment_reminder_in_subject_and_body() {
// The automatic attachment notification should pop up.
wait_for_reminder_state(cwc, true);
assert_equals(get_reminder_keywords(cwc), "attachment, attached");
// Now clear only the subject
delete_all_existing(cwc, cwc.eid("msgSubject"));
// Give the notification some time. It should not disappear.
wait_for_reminder_state(cwc, true);
// Give the notification some time. It should not disappear,
// just reduce the keywords list.
wait_for_reminder_state(cwc, true, true);
assert_equals(get_reminder_keywords(cwc), "attached");
close_compose_window(cwc);
}