stream_color: Make stream color assignment more efficient.

We now let color_data keep its own state for
unused_colors, so that we longer have to pass in
a large list of unused_colors every time we want
to assign a new stream color.

This mostly matters at startup, where we might
be cycling through 5000 streams.  We claim all
the unused colors up front.

Each operation now has an upper bound of expensiveness,
where the worst case scenario is basically popping
off the first element of a list of <= 24 colors.

The algorithm is now deterministic, too, to make
it easier to test.  It's unclear whether random color
assignment ever had much benefit, and it made unit
testing the algorithm difficult.  Now we have 100%
line coverage.

Fixes part of #10902.

frontend_tests/node_tests/color_data.js

@@ -1,13 +1,44 @@
 zrequire('color_data');
 
 run_test('pick_color', () => {
-    var used_colors = ["#76ce90", "#fae589"];
+    color_data.colors = [
+        'blue',
+        'orange',
+        'red',
+        'yellow',
+    ];
 
-    // Colors are assigned randomly, so this test is a little vague and brittle,
-    // but we can verify a few things, like not reusing colors and making sure
-    // the color has length 7.
-    var color = color_data.pick_color(used_colors);
-    assert.notEqual(color, "#76ce90");
-    assert.notEqual(color, "#fae589");
-    assert.equal(color.length, 7);
+    color_data.reset();
+
+    color_data.claim_colors([
+        { color: 'orange' },
+        { foo: 'whatever' },
+        { color: 'yellow' },
+        { color: 'bogus' },
+    ]);
+
+    var expected_colors = [
+        'blue',
+        'red',
+        // ok, now we'll cycle through all colors
+        'blue',
+        'orange',
+        'red',
+        'yellow',
+        'blue',
+        'orange',
+        'red',
+        'yellow',
+        'blue',
+        'orange',
+        'red',
+        'yellow',
+    ];
+
+    _.each(expected_colors, (expected_color) => {
+        assert.equal(color_data.pick_color(), expected_color);
+    });
+
+    color_data.claim_color('blue');
+    assert.equal(color_data.pick_color(), 'orange');
 });

frontend_tests/node_tests/stream_data.js

@@ -8,8 +8,7 @@ set_global('$', function () {
 
 set_global('blueslip', global.make_zblueslip());
 
-set_global('color_data', {});
-
+zrequire('color_data');
 zrequire('util');
 zrequire('hash_util');
 zrequire('topic_data');

static/js/archive.js

@@ -23,7 +23,7 @@ exports.initialize = function () {
     var recipient_and_topic = $('#display_recipient').html();
     var stream_name = recipient_and_topic.split('-')[0];
     var topic = recipient_and_topic.split('-')[1];
-    var recipient_color = color_data.pick_color([]);
+    var recipient_color = color_data.pick_color();
     current_message_group.message_containers = [];
     current_message_group.show_date_separator = false;
     current_message_group.display_recipient = stream_name;

static/js/color_data.js

@@ -2,9 +2,8 @@ var color_data = (function () {
 
 var exports = {};
 
-// Auto-assigned colors should be from the default palette so it's easy to undo
-// changes, so if that palette changes, change these colors.
-var stream_assignment_colors = [
+// These colors are used now for streams.
+var stream_colors = [
     "#76ce90", "#fae589", "#a6c7e5", "#e79ab5",
     "#bfd56f", "#f4ae55", "#b0a5fd", "#addfe5",
     "#f5ce6e", "#c2726a", "#94c849", "#bd86e5",
@@ -13,24 +12,50 @@ var stream_assignment_colors = [
     "#c6a8ad", "#e7cc4d", "#c8bebf", "#a47462",
 ];
 
-exports.pick_color = function (used_colors) {
-    var colors = _.shuffle(stream_assignment_colors);
-    var used_color_hash = {};
+// Shuffle our colors on page load to prevent
+// bias toward "early" colors.
+exports.colors = _.shuffle(stream_colors);
 
-    _.each(used_colors, function (color) {
-        used_color_hash[color] = true;
-    });
+exports.reset = function () {
+    exports.unused_colors = exports.colors.slice();
+};
 
-    var color = _.find(colors, function (color) {
-        return !_.has(used_color_hash, color);
-    });
+exports.reset();
 
-    if (color) {
-        return color;
+exports.claim_color = function (color) {
+    var i = exports.unused_colors.indexOf(color);
+
+    if (i < 0) {
+        return;
     }
 
-    // All available colors were used.
-    return colors[0];
+    exports.unused_colors.splice(i, 1);
+
+    if (exports.unused_colors.length === 0) {
+        exports.reset();
+    }
+};
+
+exports.claim_colors = function (subs) {
+    var used_colors = new Dict();
+
+    _.each(subs, function (sub) {
+        if (sub.color) {
+            used_colors.set(sub.color, true);
+        }
+    });
+
+    _.each(used_colors.keys(), function (color) {
+        exports.claim_color(color);
+    });
+};
+
+exports.pick_color = function () {
+    var color = exports.unused_colors[0];
+
+    exports.claim_color(color);
+
+    return color;
 };
 
 return exports;

static/js/stream_data.js

@@ -464,8 +464,7 @@ exports.create_sub_from_server_data = function (stream_name, attrs) {
     exports.set_subscribers(sub, subscriber_user_ids);
 
     if (!sub.color) {
-        var used_colors = exports.get_colors();
-        sub.color = color_data.pick_color(used_colors);
+        sub.color = color_data.pick_color();
     }
 
     exports.update_calculated_fields(sub);
@@ -552,6 +551,8 @@ exports.get_streams_for_admin = function () {
 };
 
 exports.initialize_from_page_params = function () {
+    color_data.claim_colors(page_params.subscriptions);
+
     function populate_subscriptions(subs, subscribed, previously_subscribed) {
         subs.forEach(function (sub) {
             var stream_name = sub.name;

static/js/stream_events.js

@@ -2,12 +2,6 @@ var stream_events = (function () {
 
 var exports = {};
 
-function get_color() {
-    var used_colors = stream_data.get_colors();
-    var color = color_data.pick_color(used_colors);
-    return color;
-}
-
 function update_stream_desktop_notifications(sub, value) {
     var desktop_notifications_checkbox = $(".subscription_settings[data-stream-id='" + sub.stream_id + "'] #sub_desktop_notifications_setting .sub_setting_control");
     desktop_notifications_checkbox.prop('checked', value);
@@ -108,7 +102,7 @@ exports.mark_subscribed = function (sub, subscribers, color) {
         // the backend that color.  It's not clear this code path is
         // needed.
         blueslip.warn("Frontend needed to pick a color in mark_subscribed");
-        color = get_color();
+        color = color_data.pick_color();
         subs.set_color(sub.stream_id, color);
     }
     stream_data.subscribe_myself(sub);

tools/test-js-with-node

@@ -31,6 +31,7 @@ enforce_fully_covered = {
     'static/js/buddy_data.js',
     'static/js/buddy_list.js',
     'static/js/channel.js',
+    'static/js/color_data.js',
     'static/js/colorspace.js',
     'static/js/common.js',
     'static/js/components.js',