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

Unified Diff: messageResponder.js

Issue 29339034: Issue 3869 - Use the new FilterNotifier API (Closed)
Patch Set: Created March 23, 2016, 11 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « background.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: messageResponder.js
===================================================================
--- a/messageResponder.js
+++ b/messageResponder.js
@@ -54,6 +54,7 @@
var changeListeners = new global.ext.PageMap();
var listenedPreferences = [];
+ var listenedFilterChanges = [];
var messageTypes = {
"app": "app.listen",
"filter": "filters.listen",
@@ -61,44 +62,14 @@
"subscription": "subscriptions.listen"
};
- function sendMessage(type, action, args)
+ function sendMessage(type, action)
Sebastian Noack 2016/03/23 23:15:51 Sorry for refactoring sendMessage() again, but wit
{
var pages = changeListeners.keys();
- for (var i = 0; i < pages.length; i++)
- {
- var filters = changeListeners.get(pages[i]);
- var actions = filters[type];
- if (actions && actions.indexOf(action) != -1)
- {
- pages[i].sendMessage({
- type: messageTypes[type],
- action: action,
- args: args
- });
- }
- }
- }
-
- function onFilterChange(action)
- {
- var type;
- if (action == "load")
- {
- type = "filter";
- action = "loaded";
- }
- else
- {
- var parts = action.split(".");
- type = parts[0];
- action = parts[1];
- }
-
- if (!(type in messageTypes))
+ if (pages.length == 0)
return;
var args = [];
- for (var i = 1; i < arguments.length; i++)
+ for (var i = 2; i < arguments.length; i++)
{
var arg = arguments[i];
if (arg instanceof Subscription)
@@ -109,7 +80,44 @@
args.push(arg);
}
- sendMessage(type, action, args);
+ for (var j = 0; j < pages.length; j++)
+ {
+ var page = pages[j];
+ var filters = changeListeners.get(page);
+ var actions = filters[type];
+ if (actions && actions.indexOf(action) != -1)
+ {
+ page.sendMessage({
+ type: messageTypes[type],
+ action: action,
+ args: args
+ });
+ }
+ }
+ }
+
+ function addFilterListeners(type, actions)
Sebastian Noack 2016/03/23 23:15:51 Same pattern as for Prefs.on(), the first UI that
+ {
+ actions.forEach(function(action)
+ {
+ var name;
+ if (type == "filter" && action == "loaded")
+ name = "load";
+ else
+ name = type + "." + action;
+
+ if (listenedFilterChanges.indexOf(name) == -1)
+ {
+ listenedFilterChanges.push(name);
+ FilterNotifier.on(name, function()
+ {
+ var args = [type, action];
+ for (var i = 0; i < arguments.length; i++)
Sebastian Noack 2016/03/23 23:15:51 For the record, simply passing the arguments objec
+ args.push(arguments[i]);
+ sendMessage.apply(null, args);
+ });
+ }
+ });
}
function getListenerFilters(page)
@@ -281,7 +289,7 @@
var listenerFilters = getListenerFilters(sender.page);
if (message.filter)
{
- FilterNotifier.addListener(onFilterChange);
+ addFilterListeners("filter", message.filter);
listenerFilters.filter = message.filter;
}
else
Sebastian Noack 2016/03/23 23:15:51 I wonder whether we should prehaps unsupport calli
Thomas Greiner 2016/03/24 14:22:35 Sorry, missed this comment. Yeah, in one of the p
Sebastian Noack 2016/03/24 14:40:35 So I read that as you are fine with me simplifying
@@ -312,7 +320,7 @@
listenedPreferences.push(preference);
Prefs.on(preference, function()
{
- sendMessage("pref", preference, [Prefs[preference]]);
+ sendMessage("pref", preference, Prefs[preference]);
});
}
});
@@ -338,7 +346,7 @@
{
ext.showOptions(function()
{
- sendMessage("app", "addSubscription", [convertSubscription(subscription)]);
+ sendMessage("app", "addSubscription", subscription);
});
}
else
@@ -367,7 +375,7 @@
var listenerFilters = getListenerFilters(sender.page);
if (message.filter)
{
- FilterNotifier.addListener(onFilterChange);
+ addFilterListeners("subscription", message.filter);
listenerFilters.subscription = message.filter;
}
else
@@ -383,11 +391,7 @@
if (subscription.url in FilterStorage.knownSubscriptions)
{
if (subscription.disabled || message.keepInstalled)
- {
subscription.disabled = !subscription.disabled;
- FilterNotifier.triggerListeners("subscription.disabled",
Sebastian Noack 2016/03/23 23:15:51 This belongs into the mock implementation. Otherwi
Thomas Greiner 2016/03/24 14:12:16 I agree. Well spotted.
- subscription);
- }
else
FilterStorage.removeSubscription(subscription);
}
« no previous file with comments | « background.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld