[geary/wip/794700-lazy-load-conversations: 9/19] Rework how email are loaded and displayed in the conversation viewer



commit 5c36d6493b1d23966a8da97180210bdf7c175c96
Author: Michael Gratton <mike vee net>
Date:   Wed Jan 16 14:57:11 2019 +1100

    Rework how email are loaded and displayed in the conversation viewer
    
    Ensure email are pre-loaded by the conversation list so that the viewer
    can start loading selected conversations without having to go back to
    the database. Ensure the first intereresting email is loaded and
    displayed first, then remaining email so that we get something useful
    up ASAP.

 src/client/application/geary-controller.vala       |   7 +-
 .../conversation-viewer/conversation-email.vala    | 293 ++++++++++++---------
 .../conversation-viewer/conversation-list-box.vala | 247 +++++++++--------
 .../conversation-viewer/conversation-message.vala  | 191 +++++++++-----
 4 files changed, 439 insertions(+), 299 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 4673e9cf..2908c169 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -1149,7 +1149,12 @@ public class GearyController : Geary.BaseObject {
         current_conversations = new Geary.App.ConversationMonitor(
             current_folder,
             Geary.Folder.OpenFlags.NO_DELAY,
-            ConversationListStore.REQUIRED_FIELDS,
+            // Include fields for the conversation viewer as well so
+            // conversations can be displayed without having to go
+            // back to the db
+            ConversationListStore.REQUIRED_FIELDS |
+            ConversationListBox.REQUIRED_FIELDS |
+            ConversationEmail.REQUIRED_FOR_CONSTRUCT,
             MIN_CONVERSATION_COUNT
         );
 
diff --git a/src/client/conversation-viewer/conversation-email.vala 
b/src/client/conversation-viewer/conversation-email.vala
index 2e3852ab..71d3b5fb 100644
--- a/src/client/conversation-viewer/conversation-email.vala
+++ b/src/client/conversation-viewer/conversation-email.vala
@@ -1,6 +1,6 @@
 /*
  * Copyright 2016 Software Freedom Conservancy Inc.
- * Copyright 2016 Michael Gratton <mike vee net>
+ * Copyright 2016,2019 Michael Gratton <mike vee net>
  *
  * This software is licensed under the GNU Lesser General Public License
  * (version 2.1 or later). See the COPYING file in this distribution.
@@ -21,6 +21,19 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
     // hover style isn't applied to it.
 
 
+    /** Fields that must be available for constructing the view. */
+    internal const Geary.Email.Field REQUIRED_FOR_CONSTRUCT = (
+        Geary.Email.Field.ENVELOPE |
+        Geary.Email.Field.PREVIEW |
+        Geary.Email.Field.FLAGS
+    );
+
+    /** Fields that must be available for loading the body. */
+    internal const Geary.Email.Field REQUIRED_FOR_LOAD = (
+        Geary.Email.REQUIRED_FOR_MESSAGE
+    );
+
+
     /**
      * Iterator that returns all message views in an email view.
      */
@@ -242,6 +255,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
         }
     }
 
+    /** Determines if the email is a draft message. */
+    public bool is_draft { get; private set; }
+
     /** The view displaying the email's primary message headers and body. */
     public ConversationMessage primary_message { get; private set; }
 
@@ -252,9 +268,16 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
     private Gee.List<ConversationMessage> _attached_messages =
         new Gee.LinkedList<ConversationMessage>();
 
-    /** Determines if all message's web views have finished loading. */
+    /** Determines if message bodies have started loading. */
+    public bool message_body_load_started { get; private set; default = false; }
+
+    /** Determines if all message have had loaded their bodies. */
     public bool message_bodies_loaded { get; private set; default = false; }
 
+    /** Determines if all message's web views have finished loading. */
+    private Geary.Nonblocking.Spinlock message_bodies_loaded_lock =
+        new Geary.Nonblocking.Spinlock();
+
     // Cancellable to use when loading message content
     private GLib.Cancellable load_cancellable;
 
@@ -266,12 +289,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
     // Message view with selected text, if any
     private ConversationMessage? body_selection_message = null;
 
-    // Attachment ids that have been displayed inline
-    private Gee.HashSet<string> inlined_content_ids = new Gee.HashSet<string>();
-
     // A subset of the message's attachments that are displayed in the
     // attachments view
-    private Gee.Collection<Geary.Attachment> displayed_attachments =
+    private Gee.List<Geary.Attachment> displayed_attachments =
          new Gee.LinkedList<Geary.Attachment>();
 
     // Message-specific actions
@@ -378,6 +398,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
                              GLib.Cancellable load_cancellable) {
         base_ref();
         this.email = email;
+        this.is_draft = is_draft;
         this.contact_store = contact_store;
         this.config = config;
         this.load_cancellable = load_cancellable;
@@ -436,45 +457,25 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
             });
         insert_action_group("eml", message_actions);
 
-        // Construct CID resources from attachments
-
-        Gee.Map<string,Geary.Memory.Buffer> cid_resources =
-            new Gee.HashMap<string,Geary.Memory.Buffer>();
-        foreach (Geary.Attachment att in email.attachments) {
-            if (att.content_id != null) {
-                try {
-                    cid_resources[att.content_id] =
-                        new Geary.Memory.FileBuffer(att.file, true);
-                } catch (Error err) {
-                    debug("Could not open attachment: %s", err.message);
-                }
-            }
-        }
-
         // Construct the view for the primary message, hook into it
 
-        Geary.RFC822.Message message;
-        try {
-            message = email.get_message();
-        } catch (Error error) {
-            debug("Error loading primary message: %s", error.message);
-            return;
-        }
-
         bool load_images = email.load_remote_images().is_certain();
         Geary.Contact contact = this.contact_store.get_by_rfc822(
-            message.get_primary_originator()
+            email.get_primary_originator()
         );
         if (contact != null)  {
             load_images |= contact.always_load_remote_images();
         }
 
-        this.primary_message = new ConversationMessage(message, config, load_images);
-        this.primary_message.web_view.add_internal_resources(cid_resources);
+        this.primary_message = new ConversationMessage.from_email(
+            email, load_images, config
+        );
         connect_message_view_signals(this.primary_message);
 
         this.primary_message.summary.add(this.actions);
 
+        // Wire up the rest of the UI
+
         Gtk.Builder builder = new Gtk.Builder.from_resource(
             "/org/gnome/Geary/conversation-email-menus.ui"
         );
@@ -503,8 +504,82 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
 
         pack_start(this.primary_message, true, true, 0);
         update_email_state();
+    }
+
+    ~ConversationEmail() {
+        base_unref();
+    }
+
+    /**
+     * Loads the avatar for the primary message.
+     */
+    public async void load_avatar(Application.AvatarStore store)
+        throws GLib.Error {
+        try {
+            yield this.primary_message.load_avatar(store, this.load_cancellable);
+        } catch (IOError.CANCELLED err) {
+            // okay
+        } catch (Error err) {
+            Geary.RFC822.MailboxAddress? from = this.email.get_primary_originator();
+            debug("Avatar load failed for \"%s\": %s",
+                  from != null ? from.to_string() : "<unknown>", err.message);
+        }
+    }
+
+    /**
+     * Loads the message body and attachments.
+     *
+     * This potentially hits the database if the email that the view
+     * was constructed from doesn't satisfy requirements, loads
+     * attachments, including views and avatars for any attached
+     * messages, and waits for the primary message body content to
+     * have been loaded by its web view before returning.
+     */
+    public async void load_body(Geary.App.EmailStore email_store,
+                                Application.AvatarStore contact_store)
+        throws GLib.Error {
+        this.message_body_load_started = true;
+
+        // Ensure we have required data to load the message
+
+        Geary.Email email = this.email;
+        if (!this.email.fields.fulfills(REQUIRED_FOR_LOAD)) {
+            email = yield email_store.fetch_email_async(
+                email.id,
+                Geary.Email.REQUIRED_FOR_MESSAGE,
+                Geary.Folder.ListFlags.NONE,
+                this.load_cancellable
+            );
+        }
+        Geary.RFC822.Message message = email.get_message();
+
+        // Load all mime parts and construct CID resources from them
 
-        // Add sub_messages container and message viewers if any
+        Gee.Map<string,Geary.Memory.Buffer> cid_resources =
+            new Gee.HashMap<string,Geary.Memory.Buffer>();
+        foreach (Geary.Attachment attachment in email.attachments) {
+            // Assume all parts are attachments. As the primary and
+            // secondary message bodies are loaded, any displayed
+            // inline will be removed from the list.
+            this.displayed_attachments.add(attachment);
+
+            if (attachment.content_id != null) {
+                try {
+                    cid_resources[attachment.content_id] =
+                        new Geary.Memory.FileBuffer(attachment.file, true);
+                } catch (Error err) {
+                    debug("Could not open attachment: %s", err.message);
+                }
+            }
+        }
+        this.attachments_button.set_visible(!this.displayed_attachments.is_empty);
+
+        // Load all messages
+
+        this.primary_message.web_view.add_internal_resources(cid_resources);
+        yield this.primary_message.load_message_body(
+            message, this.load_cancellable
+        );
 
         Gee.List<Geary.RFC822.Message> sub_messages = message.get_sub_messages();
         if (sub_messages.size > 0) {
@@ -512,27 +587,25 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
         }
         foreach (Geary.RFC822.Message sub_message in sub_messages) {
             ConversationMessage attached_message =
-                new ConversationMessage(sub_message, config, false);
+                new ConversationMessage.from_message(
+                    sub_message, false, this.config
+                );
             connect_message_view_signals(attached_message);
             attached_message.web_view.add_internal_resources(cid_resources);
             this.sub_messages.add(attached_message);
             this._attached_messages.add(attached_message);
-        }
-    }
-
-    ~ConversationEmail() {
-        base_unref();
-    }
-
-    /**
-     * Loads avatars for the email's messages.
-     */
-    public async void load_avatars(Application.AvatarStore store) {
-        foreach (ConversationMessage view in this)  {
-            if (!this.load_cancellable.is_cancelled()) {
-                yield view.load_avatar(store, this.load_cancellable);
+            attached_message.load_avatar.begin(
+                contact_store, this.load_cancellable
+            );
+            yield attached_message.load_message_body(
+                sub_message, this.load_cancellable
+            );
+            if (!this.is_collapsed) {
+                attached_message.show_message_body(false);
             }
         }
+
+        yield this.message_bodies_loaded_lock.wait_async(this.load_cancellable);
     }
 
     /**
@@ -562,9 +635,6 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
         this.email_menubutton.set_sensitive(true);
         foreach (ConversationMessage message in this) {
             message.show_message_body(include_transitions);
-            if (!message.body_load_started) {
-                message.load_message_body.begin(this.load_cancellable);
-            }
         }
     }
 
@@ -657,29 +727,8 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
     private void connect_message_view_signals(ConversationMessage view) {
         view.flag_remote_images.connect(on_flag_remote_images);
         view.remember_remote_images.connect(on_remember_remote_images);
-        view.web_view.internal_resource_loaded.connect((id) => {
-                this.inlined_content_ids.add(id);
-            });
-        view.web_view.notify["has-valid-height"].connect(() => {
-                bool all_loaded = true;
-                foreach (ConversationMessage message in this) {
-                    if (!message.web_view.has_valid_height) {
-                        all_loaded = false;
-                        break;
-                    }
-                }
-                if (all_loaded && !this.message_bodies_loaded) {
-                    this.message_bodies_loaded = true;
-                    // Only load attachments once the web views have
-                    // finished loading, since we want to know if any
-                    // attachments marked as being inline were
-                    // actually not displayed inline, and hence need
-                    // to be displayed as if they were attachments.
-                    if (!this.load_cancellable.is_cancelled()) {
-                        this.load_attachments.begin();
-                    }
-                }
-            });
+        view.web_view.internal_resource_loaded.connect(on_resource_loaded);
+        view.web_view.content_loaded.connect(on_content_loaded);
         view.web_view.selection_changed.connect((has_selection) => {
                 this.body_selection_message = has_selection ? view : null;
                 body_selection_changed(has_selection);
@@ -718,37 +767,10 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
         }
     }
 
-    private async void load_attachments() {
-        // Determine if we have any attachments to be displayed. This
-        // relies on the primary and any attached message bodies
-        // having being already loaded, so that we know which
-        // attachments have been shown inline and hence do not need to
-        // be included here.
-        foreach (Geary.Attachment attachment in email.attachments) {
-            if (!(attachment.content_id in this.inlined_content_ids)) {
-                Geary.Mime.DispositionType? disposition = null;
-                if (attachment.content_disposition != null) {
-                    disposition = attachment.content_disposition.disposition_type;
-                }
-                // Display both any attachment and inline parts that
-                // have already not been inlined. Although any inline
-                // parts should be referred to by other content in a
-                // multipart/related or multipart/alternative
-                // container, or inlined if in a multipart/mixed
-                // container, this cannot be not guaranteed. C.f. Bug
-                // 769868.
-                if (disposition != null &&
-                    disposition == Geary.Mime.DispositionType.ATTACHMENT ||
-                    disposition == Geary.Mime.DispositionType.INLINE) {
-                    this.displayed_attachments.add(attachment);
-                }
-            }
-        }
-
-        // Now we can actually show the attachments, if any
-        if (!this.displayed_attachments.is_empty) {
-            this.attachments_button.show();
-            this.attachments_button.set_sensitive(!this.is_collapsed);
+    private void update_displayed_attachments() {
+        bool has_attachments = !this.displayed_attachments.is_empty;
+        this.attachments_button.set_visible(has_attachments);
+        if (has_attachments) {
             this.primary_message.body_container.add(this.attachments);
 
             if (this.displayed_attachments.size > 1) {
@@ -757,12 +779,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
             }
 
             foreach (Geary.Attachment attachment in this.displayed_attachments) {
-                if (this.load_cancellable.is_cancelled()) {
-                    return;
-                }
                 AttachmentView view = new AttachmentView(attachment);
                 this.attachments_view.add(view);
-                yield view.load_icon(this.load_cancellable);
+                view.load_icon.begin(this.load_cancellable);
             }
         }
     }
@@ -858,25 +877,51 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface {
 
 
     private void on_remember_remote_images(ConversationMessage view) {
-        Geary.RFC822.MailboxAddress? sender = view.message.get_primary_originator();
-        if (sender == null) {
-            debug("Couldn't find sender for message: %s", email.id.to_string());
-            return;
+        Geary.RFC822.MailboxAddress? sender = this.email.get_primary_originator();
+        if (sender != null) {
+            Geary.Contact? contact = this.contact_store.get_by_rfc822(sender);
+            if (contact != null) {
+                Geary.ContactFlags flags = new Geary.ContactFlags();
+                flags.add(Geary.ContactFlags.ALWAYS_LOAD_REMOTE_IMAGES);
+                contact_store.mark_contacts_async.begin(
+                    Geary.Collection.single(contact), flags, null
+                );
+            }
         }
+    }
 
-        Geary.Contact? contact = contact_store.get_by_rfc822(
-            view.message.get_primary_originator()
-        );
-        if (contact == null) {
-            debug("Couldn't find contact for %s", sender.to_string());
-            return;
+    private void on_resource_loaded(string id) {
+        Gee.Iterator<Geary.Attachment> displayed =
+            this.displayed_attachments.iterator();
+        displayed.next();
+        while (displayed.has_next()) {
+            Geary.Attachment? attachment = displayed.get();
+            if (attachment.content_id == id) {
+                displayed.remove();
+            }
+            displayed.next();
         }
+    }
+
+    private void on_content_loaded() {
+        bool all_loaded = true;
+        foreach (ConversationMessage message in this) {
+            if (!message.web_view.is_content_loaded) {
+                all_loaded = false;
+                break;
+            }
+        }
+        if (all_loaded && !this.message_bodies_loaded) {
+            this.message_bodies_loaded = true;
+            this.message_bodies_loaded_lock.blind_notify();
 
-        Geary.ContactFlags flags = new Geary.ContactFlags();
-        flags.add(Geary.ContactFlags.ALWAYS_LOAD_REMOTE_IMAGES);
-        Gee.ArrayList<Geary.Contact> contact_list = new Gee.ArrayList<Geary.Contact>();
-        contact_list.add(contact);
-        contact_store.mark_contacts_async.begin(contact_list, flags, null);
+            // Update attachments once the web views have finished
+            // loading, since we want to know if any attachments
+            // marked as being inline were actually not displayed
+            // inline, and hence need to be displayed as if they were
+            // attachments.
+            this.update_displayed_attachments();
+        }
     }
 
     [GtkCallback]
diff --git a/src/client/conversation-viewer/conversation-list-box.vala 
b/src/client/conversation-viewer/conversation-list-box.vala
index a3738915..8fbcb7f1 100644
--- a/src/client/conversation-viewer/conversation-list-box.vala
+++ b/src/client/conversation-viewer/conversation-list-box.vala
@@ -1,6 +1,6 @@
 /*
  * Copyright 2016 Software Freedom Conservancy Inc.
- * Copyright 2016 Michael Gratton <mike vee net>
+ * Copyright 2016,2019 Michael Gratton <mike vee net>
  *
  * This software is licensed under the GNU Lesser General Public License
  * (version 2.1 or later). See the COPYING file in this distribution.
@@ -20,16 +20,15 @@
  */
 public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
 
-    /** Fields that must be available for display as a conversation. */
-    private const Geary.Email.Field REQUIRED_FIELDS =
-        Geary.Email.Field.HEADER
-        | Geary.Email.Field.BODY
-        | Geary.Email.Field.ORIGINATORS
-        | Geary.Email.Field.RECEIVERS
-        | Geary.Email.Field.SUBJECT
-        | Geary.Email.Field.DATE
-        | Geary.Email.Field.FLAGS
-        | Geary.ComposedEmail.REQUIRED_REPLY_FIELDS;
+    /** Fields that must be available for listing conversation email. */
+    public const Geary.Email.Field REQUIRED_FIELDS = (
+        // Sorting the conversation
+        Geary.Email.Field.DATE |
+        // Determine unread/starred, etc
+        Geary.Email.Field.FLAGS |
+        // Determine if the message is from the sender or not
+        Geary.Email.Field.ORIGINATORS
+    );
 
     // Offset from the top of the list box which emails views will
     // scrolled to, so the user can see there are additional messages
@@ -101,7 +100,10 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
         }
 
         // Request the row be expanded, if supported.
-        public virtual new void expand() {
+        public virtual new async void
+            expand(Geary.App.EmailStore email_store,
+                   Application.AvatarStore contact_store)
+            throws GLib.Error {
             // Not supported by default
         }
 
@@ -124,7 +126,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
             }
         }
 
-        protected virtual void on_size_allocate() {
+        protected void on_size_allocate() {
             // Disable should_scroll so we don't keep on scrolling
             // later, like when the window has been resized.
             this.size_allocate.disconnect(on_size_allocate);
@@ -165,14 +167,20 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
             add(view);
         }
 
-        public override void expand() {
+        public override async void
+            expand(Geary.App.EmailStore email_store,
+                   Application.AvatarStore contact_store)
+            throws GLib.Error {
             this.is_expanded = true;
+            update_row_expansion();
+            if (!this.view.message_body_load_started) {
+                yield this.view.load_body(email_store, contact_store);
+            }
             foreach (ConversationMessage message in this.view) {
                 if (!message.web_view.has_valid_height) {
                     message.web_view.queue_resize();
                 }
             };
-            update_row_expansion();
         }
 
         public override void collapse() {
@@ -181,20 +189,6 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
             update_row_expansion();
         }
 
-        protected override void on_size_allocate() {
-            // We need to wait the web view to load first, so that the
-            // message has a non-trivial height, and then wait for it
-            // to be reallocated, so that it picks up the web_view's
-            // height.
-            if (view.primary_message.web_view.has_valid_height) {
-                // Disable should_scroll after the message body has
-                // been loaded so we don't keep on scrolling later,
-                // like when the window has been resized.
-                this.size_allocate.disconnect(on_size_allocate);
-            }
-            should_scroll();
-        }
-
         private inline void update_row_expansion() {
             if (this.is_expanded || this.is_pinned) {
                 get_style_context().add_class(EXPANDED_CLASS);
@@ -433,65 +427,117 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
     }
 
     public async void load_conversation()
-        throws Error {
-        // Fetch full emails from the conversation
-        Gee.Collection<Geary.Email> full_emails =
-            yield load_full_emails(
-                this.conversation.get_emails(
-                    Geary.App.Conversation.Ordering.SENT_DATE_ASCENDING
-                )
-            );
+        throws GLib.Error {
+        set_sort_func(null);
+
+        Gee.Collection<Geary.Email>? all_email = this.conversation.get_emails(
+            Geary.App.Conversation.Ordering.SENT_DATE_ASCENDING
+        );
 
-        // Have the full set of email and a UI update is imminent. So
-        // cancel the spinner timeout if still running, and remove the
-        // spinner it may have set in any case.
+        // Now have the full set of email and a UI update is
+        // imminent. So cancel the spinner timeout if still running,
+        // and remove the spinner it may have set in any case.
         this.loading_timeout.reset();
         set_placeholder(null);
 
-        // Add them all
-        EmailRow? first_expanded_row = null;
-        foreach (Geary.Email full_email in full_emails) {
+        // Work out what the first interesting email is, and load it
+        // before all of the email before and after that so we can
+        // load them in an optimal order
+        EmailRow? first_interesting = null;
+        Gee.LinkedList<Geary.Email> uninteresting =
+            new Gee.LinkedList<Geary.Email>();
+        foreach (Geary.Email email in all_email) {
             if (this.cancellable.is_cancelled()) {
-                break;
+                throw new GLib.IOError.CANCELLED("Conversation load cancelled");
             }
-            if (!this.email_rows.has_key(full_email.id)) {
-                EmailRow row = add_email(full_email);
-                if (row.is_expanded &&
-                    (first_expanded_row == null ||
-                     on_sort(row, first_expanded_row) < 0)) {
-                    first_expanded_row = row;
+
+            if (first_interesting == null) {
+                if (email.is_unread().is_certain() ||
+                    email.is_flagged().is_certain()) {
+                    // Found and an interesting email! So load it!
+                    EmailRow row = add_email(email);
+                    first_interesting = row;
+
+                    // Update first/last row after adding the first to
+                    // avoid the UI flashing as the border
+                    // appears/disappears
+                    update_first_last_row();
+
+                    yield row.expand(this.email_store, this.avatar_store);
+                } else {
+                    // Inserted reversed so most recent uninteresting
+                    // rows are added first
+                    uninteresting.insert(0, email);
+                }
+            } else {
+                // Already found and loaded an interesting email, so
+                // load the rest now normally.
+                EmailRow row = add_email(email);
+                if (email.is_unread().is_certain() ||
+                    email.is_flagged().is_certain() ||
+                    row.view.is_draft) {
+                    yield row.expand(this.email_store, this.avatar_store);
                 }
             }
         }
 
-        update_first_last_row();
-        EmailRow? last_email = this.last_row as EmailRow;
+        if (first_interesting == null) {
+            // No interesting messages found so none have been
+            // expanded yet, show the last one at least.
+            EmailRow row = add_email(
+                uninteresting.remove_at(0)
+            );
+            // Update first/last row after adding the first to
+            // avoid the UI flashing as the border
+            // appears/disappears
+            update_first_last_row();
 
-        if (last_email != null && !this.cancellable.is_cancelled()) {
-            // If no other row was expanded by default, use the last
-            if (first_expanded_row == null) {
-                last_email.expand();
-                first_expanded_row = last_email;
-            }
+            yield row.expand(this.email_store, this.avatar_store);
+            first_interesting = row;
+        }
 
-            // Start the first expanded row loading before any others,
-            // scroll the view to it when its done
-            yield first_expanded_row.view.load_avatars(this.avatar_store);
-            first_expanded_row.should_scroll.connect(scroll_to);
-            first_expanded_row.enable_should_scroll();
+        // Finally, load all of the uninteresting messages
+        if (!uninteresting.is_empty) {
+            bool added = false;
+            Gtk.Adjustment listbox_adj = get_adjustment();
+            foreach (Geary.Email email in uninteresting) {
+                if (this.cancellable.is_cancelled()) {
+                    throw new GLib.IOError.CANCELLED(
+                        "Conversation load cancelled"
+                    );
+                }
 
-            // Start everything else loading
-            this.foreach((child) => {
-                    if (!this.cancellable.is_cancelled()) {
-                        EmailRow? row = child as EmailRow;
-                        if (row != null && row != first_expanded_row) {
-                            row.view.load_avatars.begin(this.avatar_store);
-                        }
-                    }
-                });
+                // Give GTK a moment to process newly added rows, so
+                // when updating the adjustment below the values are
+                // valid
+                GLib.Idle.add(this.load_conversation.callback);
+                yield;
+
+                EmailRow row = add_email(email, false);
+                // Drafts aren't interesting?
+                if (row.view.is_draft) {
+                    yield row.expand(this.email_store, this.avatar_store);
+                }
+
+                if (!added) {
+                    added = true;
+                    update_first_last_row();
+                }
 
-            debug("Conversation loading complete");
+                // Since uninteresting rows are inserted above the
+                // first expanded, adjust the scrollbar as they are
+                // inserted so as to keep the list scrolled to the
+                // same place.
+                row.enable_should_scroll();
+                row.should_scroll.connect(() => {
+                        Gtk.Allocation row_alloc;
+                        row.get_allocation(out row_alloc);
+                        listbox_adj.value += row_alloc.height;
+                    });
+            }
         }
+
+        set_sort_func(on_sort);
     }
 
     /**
@@ -615,8 +661,8 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
                 foreach (Geary.EmailIdentifier id in matching) {
                     EmailRow? row = this.email_rows.get(id);
                     if (row != null) {
-                        row.expand();
                         apply_search_terms(row);
+                        row.expand.begin(this.email_store, this.avatar_store);
                     }
                 }
             }
@@ -674,45 +720,31 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
             });
     }
 
-    // Given some emails, fetch the full versions with all required fields.
-    private async Gee.Collection<Geary.Email> load_full_emails(
-        Gee.Collection<Geary.Email> emails) throws Error {
-        Gee.ArrayList<Geary.EmailIdentifier> ids = new Gee.ArrayList<Geary.EmailIdentifier>();
-        foreach (Geary.Email email in emails)
-            ids.add(email.id);
-
-        Gee.Collection<Geary.Email>? full_emails =
-            yield this.email_store.list_email_by_sparse_id_async(
-                ids,
-                REQUIRED_FIELDS,
-                Geary.Folder.ListFlags.NONE,
-                this.cancellable
-            );
-
-        if (full_emails == null) {
-            full_emails = Gee.Collection.empty<Geary.Email>();
-        }
-
-        return full_emails;
-    }
-
     // Loads full version of an email, adds it to the listbox
     private async void load_full_email(Geary.EmailIdentifier id)
         throws Error {
         Geary.Email full_email = yield this.email_store.fetch_email_async(
-            id, REQUIRED_FIELDS, Geary.Folder.ListFlags.NONE, this.cancellable
+            id,
+            (
+                REQUIRED_FIELDS |
+                ConversationEmail.REQUIRED_FOR_CONSTRUCT |
+                ConversationEmail.REQUIRED_FOR_LOAD
+            ),
+            Geary.Folder.ListFlags.NONE,
+            this.cancellable
         );
 
         if (!this.cancellable.is_cancelled()) {
             EmailRow row = add_email(full_email);
-            row.expand();
             update_first_last_row();
-            yield row.view.load_avatars(this.avatar_store);
+
+            row.view.load_avatar.begin(this.avatar_store);
+            yield row.expand(this.email_store, this.avatar_store);
         }
     }
 
     // Constructs a row and view for an email, adds it to the listbox
-    private EmailRow add_email(Geary.Email email) {
+    private EmailRow add_email(Geary.Email email, bool append_row = true) {
         // Should be able to edit draft emails from any
         // conversation. This test should be more like "is in drafts
         // folder"
@@ -755,15 +787,12 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
         EmailRow row = new EmailRow(view);
         this.email_rows.set(email.id, row);
 
-        add(row);
-        email_added(view);
-
-        // Expand interesting messages by default
-        if (email.is_unread().is_certain() ||
-            email.is_flagged().is_certain() ||
-            is_draft) {
-            row.expand();
+        if (append_row) {
+            add(row);
+        } else {
+            insert(row, 0);
         }
+        email_added(view);
 
         // Apply any existing search terms to the new row
         if (this.search_terms != null) {
@@ -1023,7 +1052,7 @@ public class ConversationListBox : Gtk.ListBox, Geary.BaseInterface {
                     row.collapse();
                 }
             } else {
-                row.expand();
+                row.expand.begin(this.email_store, this.avatar_store);
             }
         }
     }
diff --git a/src/client/conversation-viewer/conversation-message.vala 
b/src/client/conversation-viewer/conversation-message.vala
index db5fabb6..ee97c671 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -1,6 +1,6 @@
 /*
  * Copyright 2016 Software Freedom Conservancy Inc.
- * Copyright 2016-2018 Michael Gratton <mike vee net>
+ * Copyright 2016-2019 Michael Gratton <mike vee net>
  *
  * This software is licensed under the GNU Lesser General Public License
  * (version 2.1 or later). See the COPYING file in this distribution.
@@ -142,12 +142,6 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
     private const string ACTION_SELECT_ALL = "select_all";
 
 
-    /** The specific RFC822 message displayed by this view. */
-    public Geary.RFC822.Message message { get; private set; }
-
-    /** Determines if the message body as started loading. */
-    public bool body_load_started { get; private set; default = false; }
-
     /** Box containing the preview and full header widgets.  */
     [GtkChild]
     internal Gtk.Grid summary;
@@ -159,6 +153,8 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
     /** HTML view that displays the message body. */
     internal ConversationWebView web_view { get; private set; }
 
+    private Geary.RFC822.MailboxAddress? primary_originator;
+
     [GtkChild]
     private Gtk.Image avatar;
 
@@ -270,18 +266,72 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
 
 
     /**
-     * Constructs a new view to display an RFC 822 message headers and body.
+     * Constructs a new view from an email's headers and body.
      *
      * This method sets up most of the user interface for displaying
      * the message, but does not attempt any possibly long-running
      * loading processes.
      */
-    public ConversationMessage(Geary.RFC822.Message message,
-                               Configuration config,
-                               bool load_remote_images) {
+    public ConversationMessage.from_email(Geary.Email email,
+                                          bool load_remote_images,
+                                          Configuration config) {
+        this(
+            email.get_primary_originator(),
+            email.from,
+            email.reply_to,
+            email.sender,
+            email.to,
+            email.cc,
+            email.bcc,
+            email.date,
+            email.subject,
+            email.preview != null ? email.preview.buffer.get_valid_utf8() : null,
+            load_remote_images,
+            config
+        );
+    }
+
+    /**
+     * Constructs a new view from an RFC 822 message's headers and body.
+     *
+     * This method sets up most of the user interface for displaying
+     * the message, but does not attempt any possibly long-running
+     * loading processes.
+     */
+    public ConversationMessage.from_message(Geary.RFC822.Message message,
+                                            bool load_remote_images,
+                                            Configuration config) {
+        this(
+            message.get_primary_originator(),
+            message.from,
+            message.reply_to,
+            message.sender,
+            message.to,
+            message.cc,
+            message.bcc,
+            message.date,
+            message.subject,
+            message.get_preview(),
+            load_remote_images,
+            config
+        );
+    }
+
+    private ConversationMessage(Geary.RFC822.MailboxAddress? primary_originator,
+                                Geary.RFC822.MailboxAddresses? from,
+                                Geary.RFC822.MailboxAddresses? reply_to,
+                                Geary.RFC822.MailboxAddress? sender,
+                                Geary.RFC822.MailboxAddresses? to,
+                                Geary.RFC822.MailboxAddresses? cc,
+                                Geary.RFC822.MailboxAddresses? bcc,
+                                Geary.RFC822.Date? date,
+                                Geary.RFC822.Subject? subject,
+                                string? preview,
+                                bool load_remote_images,
+                                Configuration config) {
         base_ref();
-        this.message = message;
         this.is_loading_images = load_remote_images;
+        this.primary_originator = primary_originator;
 
         // Actions
 
@@ -332,44 +382,50 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
         // when the message has no from address.
         string empty_from = _("No sender");
 
-        this.preview_from.set_text(format_originator_preview(empty_from));
+        this.preview_from.set_text(format_originator_preview(from, empty_from));
         this.preview_from.get_style_context().add_class(FROM_CLASS);
 
         string date_text = "";
         string date_tooltip = "";
-        if (this.message.date != null) {
+        if (date != null) {
             date_text = Date.pretty_print(
-                this.message.date.value, config.clock_format
+                date.value, config.clock_format
             );
             date_tooltip = Date.pretty_print_verbose(
-                this.message.date.value, config.clock_format
+                date.value, config.clock_format
             );
         }
         this.preview_date.set_text(date_text);
         this.preview_date.set_tooltip_text(date_tooltip);
 
-        string preview = this.message.get_preview();
-        if (preview.length > MAX_PREVIEW_BYTES) {
-            preview = Geary.String.safe_byte_substring(preview, MAX_PREVIEW_BYTES);
-            // Add an ellipsis in case the wider is wider than the text
-            preview += "…";
+        if (preview != null) {
+            string clean_preview = preview;
+            if (preview.length > MAX_PREVIEW_BYTES) {
+                clean_preview = Geary.String.safe_byte_substring(
+                    preview, MAX_PREVIEW_BYTES
+                );
+                // Add an ellipsis in case the view is wider is wider than
+                // the text
+                clean_preview += "…";
+            }
+            this.preview_body.set_text(clean_preview);
         }
-        this.preview_body.set_text(preview);
 
         // Full headers
 
-        fill_originator_addresses(empty_from);
+        fill_originator_addresses(from, reply_to, sender, empty_from);
 
         this.date.set_text(date_text);
         this.date.set_tooltip_text(date_tooltip);
-        if (this.message.subject != null) {
-            this.subject.set_text(this.message.subject.value);
+        if (subject != null) {
+            this.subject.set_text(subject.value);
             this.subject.set_visible(true);
-            this.subject_searchable = this.message.subject.value.casefold();
+            this.subject_searchable = subject.value.casefold();
         }
-        fill_header_addresses(this.to_header, this.message.to);
-        fill_header_addresses(this.cc_header, this.message.cc);
-        fill_header_addresses(this.bcc_header, this.message.bcc);
+        fill_header_addresses(this.to_header, to);
+        fill_header_addresses(this.cc_header, cc);
+        fill_header_addresses(this.bcc_header, bcc);
+
 
         // Web view
 
@@ -440,10 +496,14 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
      * Starts loading the avatar for the message's sender.
      */
     public async void load_avatar(Application.AvatarStore loader,
-                                  Cancellable load_cancelled) {
+                                  GLib.Cancellable load_cancelled)
+        throws GLib.Error {
+        if (load_cancelled.is_cancelled()) {
+            throw new GLib.IOError.CANCELLED("Conversation load cancelled");
+        }
+
         const int PIXEL_SIZE = 32;
-        Geary.RFC822.MailboxAddress? primary = message.get_primary_originator();
-        if (primary != null) {
+        if (this.primary_originator != null) {
             int window_scale = get_scale_factor();
             // We occasionally get crashes calling as below
             // Gtk.Image.get_pixel_size() when the image is
@@ -453,20 +513,15 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
             //
             //int pixel_size = this.avatar.get_pixel_size() * window_scale;
             int pixel_size = PIXEL_SIZE * window_scale;
-            try {
-                Gdk.Pixbuf? avatar_buf = yield loader.load(
-                    primary, pixel_size, load_cancelled
+            Gdk.Pixbuf? avatar_buf = yield loader.load(
+                this.primary_originator, pixel_size, load_cancelled
+            );
+            if (avatar_buf != null) {
+                this.avatar.set_from_surface(
+                    Gdk.cairo_surface_create_from_pixbuf(
+                        avatar_buf, window_scale, get_window()
+                    )
                 );
-                if (avatar_buf != null) {
-                    this.avatar.set_from_surface(
-                        Gdk.cairo_surface_create_from_pixbuf(
-                            avatar_buf, window_scale, get_window()
-                        )
-                    );
-                }
-            } catch (Error err) {
-                debug("Avatar load failed for %s: %s",
-                      primary.to_string(), err.message);
             }
         }
     }
@@ -474,13 +529,18 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
     /**
      * Starts loading the message body in the HTML view.
      */
-    public async void load_message_body(Cancellable load_cancelled) {
-        this.body_load_started = true;
+    public async void load_message_body(Geary.RFC822.Message message,
+                                        GLib.Cancellable load_cancelled)
+        throws GLib.Error {
+        if (load_cancelled.is_cancelled()) {
+            throw new GLib.IOError.CANCELLED("Conversation load cancelled");
+        }
+
         string? body_text = null;
         try {
-            body_text = (this.message.has_html_body())
-                ? this.message.get_html_body(inline_image_replacer)
-                : this.message.get_plain_body(true, inline_image_replacer);
+            body_text = (message.has_html_body())
+                ? message.get_html_body(inline_image_replacer)
+                : message.get_plain_body(true, inline_image_replacer);
         } catch (Error err) {
             debug("Could not get message text. %s", err.message);
         }
@@ -578,12 +638,12 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
         return menu;
     }
 
-    private string format_originator_preview(string empty_from_text) {
+    private string format_originator_preview(Geary.RFC822.MailboxAddresses? from,
+                                             string empty_from_text) {
         string text = "";
-        if (this.message.from != null && this.message.from.size > 0) {
+        if (from != null && from.size > 0) {
             int i = 0;
-            Gee.List<Geary.RFC822.MailboxAddress> list =
-                this.message.from.get_all();
+            Gee.List<Geary.RFC822.MailboxAddress> list = from.get_all();
             foreach (Geary.RFC822.MailboxAddress addr in list) {
                 text += addr.to_short_display();
 
@@ -599,10 +659,13 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
         return text;
     }
 
-    private void fill_originator_addresses(string empty_from_text)  {
+    private void fill_originator_addresses(Geary.RFC822.MailboxAddresses? from,
+                                           Geary.RFC822.MailboxAddresses? reply_to,
+                                           Geary.RFC822.MailboxAddress? sender,
+                                           string empty_from_text)  {
         // Show any From header addresses
-        if (this.message.from != null && this.message.from.size > 0) {
-            foreach (Geary.RFC822.MailboxAddress address in this.message.from) {
+        if (from != null && from.size > 0) {
+            foreach (Geary.RFC822.MailboxAddress address in from) {
                 AddressFlowBoxChild child = new AddressFlowBoxChild(
                     address, AddressFlowBoxChild.Type.FROM
                 );
@@ -622,10 +685,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
 
         // Show the Sender header addresses if present, but only if
         // not already in the From header.
-        if (this.message.sender != null &&
-            (this.message.from == null ||
-             !this.message.from.contains_normalized(this.message.sender.address))) {
-            AddressFlowBoxChild child = new AddressFlowBoxChild(this.message.sender);
+        if (sender != null &&
+            (from == null || !from.contains_normalized(sender.address))) {
+            AddressFlowBoxChild child = new AddressFlowBoxChild(sender);
             this.searchable_addresses.add(child);
             this.sender_header.show();
             this.sender_address.add(child);
@@ -633,10 +695,9 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface {
 
         // Show any Reply-To header addresses if present, but only if
         // each is not already in the From header.
-        if (this.message.reply_to != null) {
-            foreach (Geary.RFC822.MailboxAddress address in this.message.reply_to) {
-                if (this.message.from == null ||
-                    !this.message.from.contains_normalized(address.address)) {
+        if (reply_to != null) {
+            foreach (Geary.RFC822.MailboxAddress address in reply_to) {
+                if (from == null || !from.contains_normalized(address.address)) {
                     AddressFlowBoxChild child = new AddressFlowBoxChild(address);
                     this.searchable_addresses.add(child);
                     this.reply_to_addresses.add(child);



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]