[FIX] mail: prevent race condition in discuss

The discuss application starting process is strongly asynchronous.
However, it relies implicitely on the fact that the widget is not
destroyed in its starting process.  For example, if the widget is
detroyed before the updateChannel method is called, then any call to the
chat manager service will return undefined and the widget will crash.

When discuss is destroyed exactly between the start and the end of the
creation of its search view (which is asynchronous), then it is possible
to have a crash, because the search view will be destroyed when the
do_search method is called.

Note that we had to fix discuss mobile as well:
The _setChannel method is supposed to return a deferred, but the mobile
override did not return it.  As a result, a test in the mobile suite
failed because this.alive takes a deferred in argument.

addons/mail/static/src/js/discuss.js

@@ -170,18 +170,20 @@ var Discuss = AbstractAction.extend(ControlPanelMixin, {
         this.basicComposer.on('input_focused', this, this._onComposerFocused);
         this.extendedComposer.on('post_message', this, this._onPostMessage);
         this.extendedComposer.on('input_focused', this, this._onComposerFocused);
+        this._renderButtons();
 
         var defs = [];
-        defs.push(this._renderButtons());
         defs.push(this._renderThread());
         defs.push(this.basicComposer.appendTo(this.$('.o_mail_chat_content')));
         defs.push(this.extendedComposer.appendTo(this.$('.o_mail_chat_content')));
         defs.push(this._renderSearchView());
 
-        return $.when.apply($, defs)
-            .then(this._setChannel.bind(this, defaultChannel))
-            .then(this._updateChannels.bind(this))
+        return this.alive($.when.apply($, defs))
             .then(function () {
+                return self.alive(self._setChannel(defaultChannel));
+            })
+            .then(function () {
+                self._updateChannels();
                 self._startListening();
                 self.thread.$el.on("scroll", null, _.debounce(function () {
                     if (self.thread.get_scrolltop() < 20 &&
@@ -413,7 +415,7 @@ var Discuss = AbstractAction.extend(ControlPanelMixin, {
             disable_groupby: true,
         };
         this.searchview = new SearchView(this, this.dataset, this.fields_view, options);
-        return this.searchview.appendTo($("<div>")).then(function () {
+        return this.alive(this.searchview.appendTo($("<div>"))).then(function () {
             self.$searchview_buttons = self.searchview.$buttons.contents();
             // manually call do_search to generate the initial domain and filter
             // the messages in the default channel

addons/mail/static/src/js/discuss_mobile.js

@@ -105,9 +105,9 @@ Discuss.include({
      */
     _setChannel: function (channel) {
         if (channel.type !== 'static') {
-            this.call('chat_manager', 'detachChannel', channel.id);
+            return this.call('chat_manager', 'detachChannel', channel.id);
         } else {
-            this._super.apply(this, arguments);
+            return this._super.apply(this, arguments);
         }
     },
     /**

addons/mail/static/tests/discuss_tests.js

@@ -1,11 +1,13 @@
 odoo.define('mail.discuss_test', function (require) {
 "use strict";
 
+var Discuss = require('mail.chat_discuss');
 var ChatManager = require('mail.ChatManager');
 var mailTestUtils = require('mail.testUtils');
 
 var Bus = require('web.Bus');
 var concurrency = require('web.concurrency');
+var SearchView = require('web.SearchView');
 var testUtils = require('web.test_utils');
 
 var createBusService = mailTestUtils.createBusService;
@@ -457,6 +459,86 @@ QUnit.test('"Unstar all" button should reset the starred counter', function (ass
     });
 });
 
+QUnit.test('do not crash when destroyed before start is completed', function (assert) {
+    assert.expect(3);
+    var discuss;
+
+    testUtils.patch(Discuss, {
+        init: function () {
+            discuss = this;
+            this._super.apply(this, arguments);
+        },
+    });
+
+    createDiscuss({
+        id: 1,
+        context: {},
+        params: {},
+        data: this.data,
+        services: this.services,
+        mockRPC: function (route, args) {
+            if (args.method) {
+                assert.step(args.method);
+            }
+            var result = this._super.apply(this, arguments);
+            if (args.method === 'message_fetch') {
+                discuss.destroy();
+            }
+            return result;
+        },
+    });
+
+    assert.verifySteps([
+        "load_views",
+        "message_fetch"
+    ]);
+
+    testUtils.unpatch(Discuss);
+});
+
+QUnit.test('do not crash when destroyed between start en end of _renderSearchView', function (assert) {
+    assert.expect(2);
+    var discuss;
+
+    testUtils.patch(Discuss, {
+        init: function () {
+            discuss = this;
+            this._super.apply(this, arguments);
+        },
+    });
+
+    var def = $.Deferred();
+
+    testUtils.patch(SearchView, {
+        willStart: function () {
+            var result = this._super.apply(this, arguments);
+            return def.then($.when(result));
+        },
+    });
+
+    createDiscuss({
+        id: 1,
+        context: {},
+        params: {},
+        data: this.data,
+        services: this.services,
+        mockRPC: function (route, args) {
+            if (args.method) {
+                assert.step(args.method);
+            }
+            return this._super.apply(this, arguments);
+        },
+    });
+
+    discuss.destroy();
+    def.resolve();
+    assert.verifySteps([
+        "load_views",
+    ]);
+
+    testUtils.unpatch(Discuss);
+    testUtils.unpatch(SearchView);
+});
 
 });
 });

addons/mail/static/tests/helpers/test_utils.js

@@ -78,7 +78,6 @@ function createDiscuss(params) {
     var selector = params.debug ? 'body' : '#qunit-fixture';
     var controlPanel = new ControlPanel(parent);
     controlPanel.appendTo($(selector));
-    discuss.appendTo($(selector));
 
     // override 'destroy' of discuss so that it calls 'destroy' on the parent
     // instead, which is the parent of discuss and the mockServer.
@@ -92,7 +91,7 @@ function createDiscuss(params) {
     // link the view to the control panel
     discuss.set_cp_bus(controlPanel.get_bus());
 
-    return discuss.call('chat_manager', 'isReady').then(function () {
+    return  discuss.appendTo($(selector)).then(function () {
         return discuss;
     });
 }