perf(drafts): Cache results of running Autolinker, avoid props.children for EmailFrame, fix special case for new draft

Summary:
"results.length" not "result.length".

Eventually I want MessageBodyProcessor to be the public interface to another process which asynchronously runs message body parsing? At the very least, caching the results means that miscelaneous refreshes to the message list don't incur significant string processing cost.

Test Plan: No new tests here

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D1898

exports/nylas-exports.coffee

@@ -62,6 +62,7 @@ Exports =
   DraftCountStore: require '../src/flux/stores/draft-count-store'
   DraftStoreExtension: require '../src/flux/stores/draft-store-extension'
   MessageStore: require '../src/flux/stores/message-store'
+  MessageBodyProcessor: require '../src/flux/stores/message-body-processor'
   EventStore: require '../src/flux/stores/event-store'
   MessageStoreExtension: require '../src/flux/stores/message-store-extension'
   ContactStore: require '../src/flux/stores/contact-store'

internal_packages/message-list/lib/email-frame.cjsx

@@ -6,6 +6,9 @@ _ = require "underscore"
 class EmailFrame extends React.Component
   @displayName = 'EmailFrame'
 
+  @propTypes:
+    content: React.PropTypes.string.isRequired
+
   render: =>
     <EventedIFrame ref="iframe" seamless="seamless" />
 
@@ -57,13 +60,11 @@ class EmailFrame extends React.Component
         @_setFrameHeight()
 
   _emailContent: =>
-    email = @props.children
-
     # When showing quoted text, always return the pure content
     if @props.showQuotedText
-      email
+      @props.content
     else
-      QuotedHTMLParser.hideQuotedHTML(email)
+      QuotedHTMLParser.hideQuotedHTML(@props.content)
 
 
 module.exports = EmailFrame

internal_packages/message-list/lib/message-item.cjsx

@@ -10,6 +10,7 @@ MessageControls = require './message-controls'
  MessageUtils,
  NamespaceStore,
  MessageStore,
+ MessageBodyProcessor,
  QuotedHTMLParser,
  ComponentRegistry,
  FileDownloadStore} = require 'nylas-exports'
@@ -18,7 +19,6 @@ MessageControls = require './message-controls'
  InjectedComponent} = require 'nylas-component-kit'
 
 TransparentPixel = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR4nGNikAQAACIAHF/uBd8AAAAASUVORK5CYII="
-MessageBodyWidth = 740
 
 class MessageItem extends React.Component
   @displayName = 'MessageItem'
@@ -79,9 +79,7 @@ class MessageItem extends React.Component
       <div className="message-item-white-wrap">
         <div className="message-item-area">
           {@_renderHeader()}
-          <EmailFrame showQuotedText={@state.showQuotedText}>
-            {@_formatBody()}
-          </EmailFrame>
+          <EmailFrame showQuotedText={@state.showQuotedText} content={@_formatBody()}/>
           <a className={@_quotedTextClasses()} onClick={@_toggleQuotedText}></a>
           {@_renderEvents()}
           {@_renderAttachments()}
@@ -206,33 +204,8 @@ class MessageItem extends React.Component
   _formatBody: =>
     return "" unless @props.message and @props.message.body
 
-    # Give each extension the message object to process the body, but don't
-    # allow them to modify anything but the body for the time being.
-    body = @props.message.body
-    for extension in MessageStore.extensions()
-      continue unless extension.formatMessageBody
-      virtual = @props.message.clone()
-      virtual.body = body
-      extension.formatMessageBody(virtual)
-      body = virtual.body
-
-    # Find inline images and give them a calculated CSS height based on
-    # html width and height, when available. This means nothing changes size
-    # as the image is loaded, and we can estimate final height correctly.
-    # Note that MessageBodyWidth must be updated if the UI is changed!
-
-    while (result = MessageUtils.cidRegex.exec(body)) isnt null
-      imgstart = body.lastIndexOf('<', result.index)
-      imgend = body.indexOf('/>', result.index)
-
-      if imgstart != -1 and imgend > imgstart
-        imgtag = body.substr(imgstart, imgend - imgstart)
-        width = imgtag.match(/width[ ]?=[ ]?['"]?(\d*)['"]?/)?[1]
-        height = imgtag.match(/height[ ]?=[ ]?['"]?(\d*)['"]?/)?[1]
-        if width and height
-          scale = Math.min(1, MessageBodyWidth / width)
-          style = " style=\"height:#{height * scale}px;\" "
-          body = body.substr(0, imgend) + style + body.substr(imgend)
+    # Runs extensions, potentially asynchronous soon
+    body = MessageBodyProcessor.process(@props.message.id, @props.message)
 
     # Replace cid:// references with the paths to downloaded files
     for file in @props.message.files

internal_packages/message-list/spec/message-item-spec.cjsx

@@ -8,7 +8,8 @@ ReactTestUtils = React.addons.TestUtils
  Thread,
  Utils,
  QuotedHTMLParser,
- FileDownloadStore} = require "nylas-exports"
+ FileDownloadStore,
+ MessageBodyProcessor} = require "nylas-exports"
 
 EmailFrameStub = React.createClass({render: -> <div></div>})
 
@@ -97,6 +98,8 @@ describe "MessageItem", ->
     spyOn(FileDownloadStore, 'downloadDataForFiles').andCallFake (ids) ->
       return {'file_1_id': download, 'file_inline_downloading_id': download_inline}
 
+    spyOn(MessageBodyProcessor, 'addToCache').andCallFake ->
+
     @message = new Message
       id: "111"
       from: [user_1]

internal_packages/thread-list/lib/thread-list-quick-actions.cjsx

@@ -13,7 +13,7 @@ class ThreadListQuickActions extends React.Component
   @displayName: 'ThreadListQuickActions'
   @propTypes:
     thread: React.PropTypes.object
-    categoryId: React.PropTypes.integer
+    categoryId: React.PropTypes.string
 
   render: =>
     actions = []

src/browser/application.coffee

@@ -123,6 +123,28 @@ class Application
         # which is why this check is here.
         throw error unless error.code is 'ENOENT'
 
+  # On Windows, removing a file can fail if a process still has it open. When
+  # we close windows and log out, we need to wait for these processes to completely
+  # exit and then delete the file. It's hard to tell when this happens, so we just
+  # retry the deletion a few times.
+  deleteFileWithRetry: (filePath, callback, retries = 5) ->
+    callbackWithRetry = (err) =>
+      if err
+        console.log("File Error: #{err.message} - retrying in 150msec")
+        setTimeout =>
+          @deleteFileWithRetry(filePath, callback, retries - 1)
+        , 150
+      else
+        callback(null)
+
+    if not fs.existsSync(filePath)
+      callback(null)
+
+    if retries > 0
+      fs.unlink(filePath, callbackWithRetry)
+    else
+      fs.unlink(filePath, callback)
+
   # Configures required javascript environment flags.
   setupJavaScriptArguments: ->
     app.commandLine.appendSwitch 'js-flags', '--harmony'
@@ -131,7 +153,7 @@ class Application
     @setDatabasePhase('close')
     @windowManager.closeMainWindow()
     @windowManager.unregisterAllHotWindows()
-    fs.unlink path.join(configDirPath,'edgehill.db'), =>
+    @deleteFileWithRetry path.join(configDirPath,'edgehill.db'), =>
       @config.set('nylas', null)
       @config.set('edgehill', null)
       @setDatabasePhase('setup')
@@ -156,7 +178,7 @@ class Application
       message: 'Upgrading Nylas'
       detail: 'Welcome back to Nylas! We need to rebuild your mailbox to support new features. Please wait a few moments while we re-sync your mail.'
       buttons: ['OK']
-    fs.unlink path.join(configDirPath,'edgehill.db'), (err) =>
+    @deleteFileWithRetry path.join(configDirPath,'edgehill.db'), =>
       @setDatabasePhase('setup')
       @windowManager.showMainWindow()
 

src/flux/stores/database-store.coffee

@@ -196,7 +196,7 @@ class DatabaseStore extends NylasStore
           console.error("DatabaseStore: Query #{query}, #{JSON.stringify(values)} failed #{err.toString()}")
         else
           duration = Date.now() - start
-          metadata = {duration: duration, resultLength: result?.length}
+          metadata = {duration: duration, resultLength: results?.length}
           console.debug(DEBUG_TO_LOG, "DatabaseStore: END (#{duration}) #{query}", metadata)
 
         if query is COMMIT

src/flux/stores/message-body-processor.coffee

@@ -0,0 +1,60 @@
+MessageUtils = require '../models/message-utils'
+MessageStore = require './message-store'
+
+MessageBodyWidth = 740
+
+class MessageBodyProcessor
+
+  constructor: ->
+    @resetCache()
+
+  resetCache: ->
+    # Store an object for recently processed items. Put the item reference into
+    # both data structures so we can access it in O(1) and also delete in O(1)
+    @_recentlyProcessedA = []
+    @_recentlyProcessedD = {}
+
+  process: (key, message) ->
+    body = message.body
+    if @_recentlyProcessedD[key]
+      return @_recentlyProcessedD[key].body
+
+    # Give each extension the message object to process the body, but don't
+    # allow them to modify anything but the body for the time being.
+    for extension in MessageStore.extensions()
+      continue unless extension.formatMessageBody
+      virtual = message.clone()
+      virtual.body = body
+      extension.formatMessageBody(virtual)
+      body = virtual.body
+
+    # Find inline images and give them a calculated CSS height based on
+    # html width and height, when available. This means nothing changes size
+    # as the image is loaded, and we can estimate final height correctly.
+    # Note that MessageBodyWidth must be updated if the UI is changed!
+
+    while (result = MessageUtils.cidRegex.exec(body)) isnt null
+      imgstart = body.lastIndexOf('<', result.index)
+      imgend = body.indexOf('/>', result.index)
+
+      if imgstart != -1 and imgend > imgstart
+        imgtag = body.substr(imgstart, imgend - imgstart)
+        width = imgtag.match(/width[ ]?=[ ]?['"]?(\d*)['"]?/)?[1]
+        height = imgtag.match(/height[ ]?=[ ]?['"]?(\d*)['"]?/)?[1]
+        if width and height
+          scale = Math.min(1, MessageBodyWidth / width)
+          style = " style=\"height:#{height * scale}px;\" "
+          body = body.substr(0, imgend) + style + body.substr(imgend)
+
+    @addToCache(key, body)
+    body
+
+  addToCache: (key, body) ->
+    if @_recentlyProcessedA.length > 50
+      removed = @_recentlyProcessedA.pop()
+      delete @_recentlyProcessedD[removed.key]
+    item = {key, body}
+    @_recentlyProcessedA.unshift(item)
+    @_recentlyProcessedD[key] = item
+
+module.exports = new MessageBodyProcessor()

src/flux/stores/message-store.coffee

@@ -53,6 +53,8 @@ class MessageStore extends NylasStore
   #
   registerExtension: (ext) =>
     @_extensions.push(ext)
+    MessageBodyProcessor = require './message-body-processor'
+    MessageBodyProcessor.resetCache()
 
   # Public: Unregisters the extension provided from the MessageStore.
   #
@@ -60,6 +62,8 @@ class MessageStore extends NylasStore
   #
   unregisterExtension: (ext) =>
     @_extensions = _.without(@_extensions, ext)
+    MessageBodyProcessor = require './message-body-processor'
+    MessageBodyProcessor.resetCache()
 
 
   ########### PRIVATE ####################################################
@@ -84,18 +88,23 @@ class MessageStore extends NylasStore
     if change.objectClass is Message.name
       inDisplayedThread = _.some change.objects, (obj) => obj.threadId is @_thread.id
       if inDisplayedThread
-        @_fetchFromCache()
 
         # Are we most likely adding a new draft? If the item is a draft and we don't
         # have it's local Id, optimistically add it to the set, resort, and trigger.
         # Note: this can avoid 100msec+ of delay from "Reply" => composer onscreen,
         item = change.objects[0]
-        if change.objects.length is 1 and item.draft is true and @_itemsLocalIds[item.id] is null
+        itemAlreadyExists = _.some @_items, (msg) -> msg.id is item.id
+        if change.objects.length is 1 and item.draft is true and not itemAlreadyExists
           DatabaseStore.localIdForModel(item).then (localId) =>
             @_itemsLocalIds[item.id] = localId
-            @_items.push(item)
+            # We need to create a new copy of the items array so that the message-list
+            # can compare new state to previous state.
+            @_items = [].concat(@_items, [item])
             @_items = @_sortItemsForDisplay(@_items)
+            @_expandItemsToDefault()
             @trigger()
+        else
+          @_fetchFromCache()
 
     if change.objectClass is Thread.name
       updatedThread = _.find change.objects, (obj) => obj.id is @_thread.id

src/flux/tasks/destroy-draft.coffee

@@ -35,7 +35,7 @@ class DestroyDraftTask extends Task
     # when we performed locally, or if the draft has never been synced to
     # the server (id is still self-assigned)
     return Promise.resolve(Task.Status.Finished) unless @draft
-    return Promise.resolve(Task.Status.Finished) unless @draft.isSaved()
+    return Promise.resolve(Task.Status.Finished) unless @draft.isSaved() and @draft.version
 
     NylasAPI.makeRequest
       path: "/n/#{@draft.namespaceId}/drafts/#{@draft.id}"