perf(thread-list): Tailored SQLite indexes, ListTabular / ScrollRegion optimizations galore

Summary:
Allow Database models to create indexes, but don't autocreate bad ones

fix minor bug in error-reporter

Fix index on message list to make thread list lookups use proper index

Developer bar ignores state changes unless it's open

DatabaseView now asks for metadata for a set of items rather than calling a function for every item. Promise.props was cute but we really needed to make a single database query for all message metadata.

New "in" matcher so you can say `thread_id IN (1,2,3)`

Add .scroll-region-content-inner which is larger than the viewport by 1 page size, and uses transform(0,0,0) trick

ScrollRegion exposes `onScrollEnd` so listTabular, et al don't need to re-implement it with more timers. Also removing requestAnimationFrame which was causing us to request scrollTop when it was not ready, and caching the values of...

...clientHeight/scrollHeight while scrolling is in-flight

Updating rendered content 10 rows at a time (RangeChunkSize) was a bad idea. Instead, add every row in a render: pass as it comes in (less work all the time vs more work intermittently). Also remove bad requestAnimationFrame, and prevent calls to...

...updateRangeState from triggering additional calls to updateRangeState by removing `componentDidUpdate => updateRangeState `

Turning off hover (pointer-events:none) is now standard in ScrollRegion

Loading text in the scroll tooltip, instead of random date shown

Handle query parse errors by catching error and throwing a better more explanatory error

Replace "quick action" retina images with background images to make React render easier

Replace hasTagId with a faster implementation which doesn't call functions and doesn't build a temporary array

Print query durations when printing to console instead of only in metadata

Remove headers from support from ListTabular, we'll never use it

Making columns part of state was a good idea but changing the array causes the entire ListTabular to re-render.  To avoid this, be smarter about updating columns. This logic could potentially go in `componentDidReceiveProps` too.

Fix specs and add 6 more for new database store functionality

Test Plan: Run 6 new specs. More in the works?

Reviewers: evan

Reviewed By: evan

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

internal_packages/developer-bar/lib/developer-bar.cjsx

@@ -128,7 +128,11 @@ class DeveloperBar extends React.Component
       expandedDiv
 
   _onChange: =>
-    @setState(@_getStateFromStores())
+    # The developer bar is hidden almost all the time. Rather than render when
+    # API requests come in, etc., just ignore changes from our store and retrieve
+    # state when we open.
+    if @state.visible and @state.height > DeveloperBarClosedHeight
+      @setState(@_getStateFromStores())
 
   _onClear: =>
     Actions.clearDeveloperConsole()
@@ -144,9 +148,11 @@ class DeveloperBar extends React.Component
       height: DeveloperBarClosedHeight
 
   _onShow: =>
+    @setState(@_getStateFromStores())
     @setState(height: 200) if @state.height < 100
 
   _onExpandSection: (section) =>
+    @setState(@_getStateFromStores())
     @setState(section: section)
     @_onShow()
 

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

@@ -5,7 +5,6 @@ React = require 'react'
  Thread,
  AddRemoveTagsTask,
  NamespaceStore} = require 'nylas-exports'
-{RetinaImg} = require 'nylas-component-kit'
 
 class ThreadListQuickActions extends React.Component
   @displayName: 'ThreadListQuickActions'
@@ -14,10 +13,10 @@ class ThreadListQuickActions extends React.Component
 
   render: =>
     actions = []
-    actions.push <div className="action" onClick={@_onReply}><RetinaImg name="toolbar-reply.png" mode={RetinaImg.Mode.ContentPreserve} /></div>
+    actions.push <div key="reply" className="action action-reply" onClick={@_onReply}></div>
-    actions.push <div className="action" onClick={@_onForward}><RetinaImg name="toolbar-forward.png" mode={RetinaImg.Mode.ContentPreserve} /></div>
+    actions.push <div key="fwd" className="action action-forward" onClick={@_onForward}></div>
     if not @props.thread.hasTagId('archive')
-      actions.push <div className="action" onClick={@_onArchive}><RetinaImg name="toolbar-archive.png" mode={RetinaImg.Mode.ContentPreserve} /></div>
+      actions.push <div key="archive" className="action action-archive" onClick={@_onArchive}></div>
 
     <div className="inner">
       {actions}

internal_packages/thread-list/lib/thread-list-store.coffee

@@ -70,8 +70,15 @@ ThreadListStore = Reflux.createStore
       matchers = []
       matchers.push Thread.attributes.namespaceId.equal(namespaceId)
       matchers.push Thread.attributes.tags.contains(tagId) if tagId isnt "*"
-      @setView new DatabaseView Thread, {matchers}, (item) ->
+      view = new DatabaseView Thread, {matchers}, (ids) =>
-        DatabaseStore.findAll(Message, {threadId: item.id})
+        DatabaseStore.findAll(Message).where(Message.attributes.threadId.in(ids)).then (messages) ->
+          messagesByThread = {}
+          for id in ids
+            messagesByThread[id] = []
+          for message in messages
+            messagesByThread[message.threadId].push message
+          messagesByThread
+      @setView(view)
 
     Actions.setFocus(collection: 'thread', item: null)
 

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

@@ -38,8 +38,12 @@ class ThreadListScrollTooltip extends React.Component
       item: ThreadListStore.view().get(idx)
 
   render: ->
+    if @state.item
+      content = timestamp(@state.item.lastMessageTimestamp)
+    else
+      content = "Loading..."
     <div className="scroll-tooltip">
-      {timestamp(@state.item?.lastMessageTimestamp)}
+      {content}
     </div>
 
 class ThreadList extends React.Component

internal_packages/thread-list/stylesheets/thread-list.less

@@ -185,11 +185,24 @@
 .thread-list .list-item .list-column-HoverActions {
   display:none;
   .action {
-    margin:8px;
     display:inline-block;
+    background-size: 100%;
+    zoom:0.5;
+    width:48px;
+    height:48px;
+    margin:16px;
   }
   .action:last-child {
-    margin-right:20px;
+    margin-right:40px;
+  }
+  .action.action-reply {
+    background: url(../static/images/toolbar/toolbar-reply@2x.png) top left no-repeat;
+  }
+  .action.action-forward {
+    background: url(../static/images/toolbar/toolbar-forward@2x.png) top left no-repeat;
+  }
+  .action.action-archive {
+    background: url(../static/images/toolbar/toolbar-archive@2x.png) top left no-repeat;
   }
 }
 .thread-list .list-item:hover .list-column-HoverActions {

spec-nylas/database-view-spec.coffee

@@ -41,7 +41,7 @@ describe "DatabaseView", ->
     it "should optionally accept a metadata provider", ->
       provider = ->
       view = new DatabaseView(Message, {}, provider)
-      expect(view._itemMetadataProvider).toEqual(provider)
+      expect(view._metadataProvider).toEqual(provider)
 
     it "should initialize the row count to -1", ->
       view = new DatabaseView(Message)
@@ -73,9 +73,9 @@ describe "DatabaseView", ->
       @view._count = 1
       spyOn(@view, 'invalidateRetainedRange').andCallFake ->
 
-    describe "setItemMetadataProvider", ->
+    describe "setMetadataProvider", ->
       it "should empty the page cache and re-fetch all pages", ->
-        @view.setItemMetadataProvider( -> false)
+        @view.setMetadataProvider( -> false)
         expect(@view._pages).toEqual({})
         expect(@view.invalidateRetainedRange).toHaveBeenCalled()
 
@@ -317,8 +317,11 @@ describe "DatabaseView", ->
 
       describe "if an item metadata provider is configured", ->
         beforeEach ->
-          @view._itemMetadataProvider = (item) ->
+          @view._metadataProvider = (ids) ->
-            Promise.resolve('metadata-for-'+item.id)
+            results = {}
+            for id in ids
+              results[id] = "metadata-for-#{id}"
+            Promise.resolve(results)
 
         it "should set .metadata of each item", ->
           runs ->
@@ -342,9 +345,12 @@ describe "DatabaseView", ->
 
         it "should always wait for metadata promises to resolve", ->
           @resolves = []
-          @view._itemMetadataProvider = (item) =>
+          @view._metadataProvider = (ids) =>
             new Promise (resolve, reject) =>
-              @resolves.push -> resolve('metadata-for-'+item.id)
+              results = {}
+              for id in ids
+                results[id] = "metadata-for-#{id}"
+              @resolves.push -> resolve(results)
 
           runs ->
             @completeQuery()

spec-nylas/stores/database-store-spec.coffee

@@ -12,6 +12,7 @@ class TestModel extends Model
       modelKey: 'id'
 
 TestModel.configureWithAllAttributes = ->
+  TestModel.additionalSQLiteConfig = undefined
   TestModel.attributes =
     'datetime': Attributes.DateTime
       queryable: true
@@ -30,6 +31,7 @@ TestModel.configureWithAllAttributes = ->
       modelKey: 'other'
 
 TestModel.configureWithCollectionAttribute = ->
+  TestModel.additionalSQLiteConfig = undefined
   TestModel.attributes =
     'id': Attributes.String
       queryable: true
@@ -41,6 +43,7 @@ TestModel.configureWithCollectionAttribute = ->
 
 
 TestModel.configureWithJoinedDataAttribute = ->
+  TestModel.additionalSQLiteConfig = undefined
   TestModel.attributes =
     'id': Attributes.String
       queryable: true
@@ -50,6 +53,22 @@ TestModel.configureWithJoinedDataAttribute = ->
       modelKey: 'body'
 
 
+TestModel.configureWithAdditionalSQLiteConfig = ->
+  TestModel.attributes =
+    'id': Attributes.String
+      queryable: true
+      modelKey: 'id'
+    'body': Attributes.JoinedData
+      modelTable: 'TestModelBody'
+      modelKey: 'body'
+  TestModel.additionalSQLiteConfig =
+    setup: ->
+      ['CREATE INDEX IF NOT EXISTS ThreadListIndex ON Thread(last_message_timestamp DESC, namespace_id, id)']
+    writeModel: jasmine.createSpy('additionalWriteModel')
+    deleteModel: jasmine.createSpy('additionalDeleteModel')
+
+
+
 testMatchers = {'id': 'b'}
 testModelInstance = new TestModel(id: '1234')
 testModelInstanceA = new TestModel(id: 'AAA')
@@ -161,6 +180,19 @@ describe "DatabaseStore", ->
       change = DatabaseStore.triggerSoon.mostRecentCall.args[0]
       expect(change).toEqual({objectClass: TestModel.name, objects: [testModelInstance], type:'unpersist'})
 
+    describe "when the model provides additional sqlite config", ->
+      beforeEach ->
+        TestModel.configureWithAdditionalSQLiteConfig()
+
+      it "should call the deleteModel method and provide the transaction and model", ->
+        DatabaseStore.unpersistModel(testModelInstance)
+        expect(TestModel.additionalSQLiteConfig.deleteModel).toHaveBeenCalled()
+        expect(TestModel.additionalSQLiteConfig.deleteModel.mostRecentCall.args[1]).toBe(testModelInstance)
+
+      it "should not fail if additional config is present, but deleteModel is not defined", ->
+        delete TestModel.additionalSQLiteConfig['deleteModel']
+        expect( => DatabaseStore.unpersistModel(testModelInstance)).not.toThrow()
+
     describe "when the model has collection attributes", ->
       it "should delete all of the elements in the join tables", ->
         TestModel.configureWithCollectionAttribute()
@@ -178,7 +210,7 @@ describe "DatabaseStore", ->
         expect(@performed[2].values[0]).toBe('1234')
 
   describe "queriesForTableSetup", ->
-    it "should return the queries for creating the table and indexes on queryable columns", ->
+    it "should return the queries for creating the table and the primary unique index", ->
       TestModel.attributes =
         'attrQueryable': Attributes.DateTime
           queryable: true
@@ -191,7 +223,6 @@ describe "DatabaseStore", ->
       queries = DatabaseStore.queriesForTableSetup(TestModel)
       expected = [
         'CREATE TABLE IF NOT EXISTS `TestModel` (id TEXT PRIMARY KEY,data BLOB,attr_queryable INTEGER)',
-        'CREATE INDEX IF NOT EXISTS `TestModel_attr_queryable` ON `TestModel` (`attr_queryable`)',
         'CREATE UNIQUE INDEX IF NOT EXISTS `TestModel_id` ON `TestModel` (`id`)'
       ]
       for query,i in queries
@@ -214,6 +245,19 @@ describe "DatabaseStore", ->
       queries = DatabaseStore.queriesForTableSetup(TestModel)
       expect(queries[0]).toBe('CREATE TABLE IF NOT EXISTS `TestModel` (id TEXT PRIMARY KEY,data BLOB,datetime INTEGER,string-json-key TEXT,boolean INTEGER,number INTEGER)')
 
+    describe "when the model provides additional sqlite config", ->
+      it "the setup method should return these queries", ->
+        TestModel.configureWithAdditionalSQLiteConfig()
+        spyOn(TestModel.additionalSQLiteConfig, 'setup').andCallThrough()
+        queries = DatabaseStore.queriesForTableSetup(TestModel)
+        expect(TestModel.additionalSQLiteConfig.setup).toHaveBeenCalledWith()
+        expect(queries.pop()).toBe('CREATE INDEX IF NOT EXISTS ThreadListIndex ON Thread(last_message_timestamp DESC, namespace_id, id)')
+
+      it "should not fail if additional config is present, but setup is undefined", ->
+        delete TestModel.additionalSQLiteConfig['setup']
+        @m = new TestModel(id: 'local-6806434c-b0cd', body: 'hello world')
+        expect( => DatabaseStore.queriesForTableSetup(TestModel)).not.toThrow()
+
   describe "writeModels", ->
     it "should compose a REPLACE INTO query to save the model", ->
       TestModel.configureWithCollectionAttribute()
@@ -276,3 +320,18 @@ describe "DatabaseStore", ->
         @m = new TestModel(id: 'local-6806434c-b0cd')
         DatabaseStore.writeModels(@spyTx(), [@m])
         expect(@performed.length).toBe(1)
+
+    describe "when the model provides additional sqlite config", ->
+      beforeEach ->
+        TestModel.configureWithAdditionalSQLiteConfig()
+
+      it "should call the writeModel method and provide the transaction and model", ->
+        tx = @spyTx()
+        @m = new TestModel(id: 'local-6806434c-b0cd', body: 'hello world')
+        DatabaseStore.writeModels(tx, [@m])
+        expect(TestModel.additionalSQLiteConfig.writeModel).toHaveBeenCalledWith(tx, @m)
+
+      it "should not fail if additional config is present, but writeModel is not defined", ->
+        delete TestModel.additionalSQLiteConfig['writeModel']
+        @m = new TestModel(id: 'local-6806434c-b0cd', body: 'hello world')
+        expect( => DatabaseStore.writeModels(@spyTx(), [@m])).not.toThrow()

src/components/list-tabular.cjsx

@@ -3,8 +3,6 @@ React = require 'react/addons'
 ScrollRegion = require './scroll-region'
 {Utils} = require 'nylas-exports'
 
-RangeChunkSize = 10
-
 class ListColumn
   constructor: ({@name, @resolver, @flex, @width}) ->
 
@@ -15,7 +13,6 @@ class ListTabularItem extends React.Component
     columns: React.PropTypes.arrayOf(React.PropTypes.object).isRequired
     item: React.PropTypes.object.isRequired
     itemProps: React.PropTypes.object
-    displayHeaders: React.PropTypes.bool
     onSelect: React.PropTypes.func
     onClick: React.PropTypes.func
     onDoubleClick: React.PropTypes.func
@@ -78,41 +75,24 @@ class ListTabular extends React.Component
     @state =
       renderedRangeStart: -1
       renderedRangeEnd: -1
-      scrollTop: 0
-      scrollInProgress: false
 
   componentDidMount: =>
     @updateRangeState()
 
-  componentWillUnmount: =>
-    clearTimeout(@_scrollTimer) if @_scrollTimer
-
   componentDidUpdate: (prevProps, prevState) =>
     # If our view has been swapped out for an entirely different one,
     # reset our scroll position to the top.
     if prevProps.dataView isnt @props.dataView
       @refs.container.scrollTop = 0
-    @updateRangeState()
 
-  updateScrollState: =>
+    unless @updateRangeStateFiring
-    window.requestAnimationFrame =>
+      @updateRangeState()
-      # Create an event that fires when we stop receiving scroll events.
+    @updateRangeStateFiring = false
-      # There is no "scrollend" event, but we really need one.
-      clearTimeout(@_scrollTimer) if @_scrollTimer
-      @_scrollTimer = setTimeout(@onDoneReceivingScrollEvents, 100)
 
-      # If we just started scrolling, scrollInProgress changes our CSS styles
-      # and disables pointer events to our contents for rendering speed
-      @setState({scrollInProgress: true}) unless @state.scrollInProgress
 
-      # If we've shifted enough pixels from our previous scrollTop to require
+  onScroll: =>
-      # new rows to be rendered, update our state!
+    # If we've shifted enough pixels from our previous scrollTop to require
-      if Math.abs(@state.scrollTop - @refs.container.scrollTop) >= @props.itemHeight * RangeChunkSize
+    # new rows to be rendered, update our state!
-        @updateRangeState()
-
-  onDoneReceivingScrollEvents: =>
-    return unless React.findDOMNode(@refs.container)
-    @setState({scrollInProgress: false})
     @updateRangeState()
 
   updateRangeState: =>
@@ -120,66 +100,42 @@ class ListTabular extends React.Component
 
     # Determine the exact range of rows we want onscreen
     rangeStart = Math.floor(scrollTop / @props.itemHeight)
-    rangeEnd = rangeStart + window.innerHeight / @props.itemHeight
+    rangeSize = Math.ceil(window.innerHeight / @props.itemHeight)
+    rangeEnd = rangeStart + rangeSize
 
     # 1. Clip this range to the number of available items
     #
-    # 2. Expand the range by more than RangeChunkSize so that
+    # 2. Expand the range by a bit so that we prepare items offscreen
-    #    the user can scroll through RangeChunkSize more items before
+    #    before they're seen. This works because we force a compositor
-    #    another render is required.
+    #    layer using transform:translate3d(0,0,0)
     #
-    rangeStart = Math.max(0, rangeStart - RangeChunkSize * 1.5)
+    rangeStart = Math.max(0, rangeStart - rangeSize)
-    rangeEnd = Math.min(rangeEnd + RangeChunkSize * 1.5, @props.dataView.count())
+    rangeEnd = Math.min(rangeEnd + rangeSize, @props.dataView.count())
-
-    if @state.scrollInProgress
-      # only extend the range while scrolling. If we remove the DOM node
-      # the user started scrolling over, the deceleration stops.
-      # https://code.google.com/p/chromium/issues/detail?id=312427
-      if @state.renderedRangeStart != -1
-        rangeStart = Math.min(@state.renderedRangeStart, rangeStart)
-      if @state.renderedRangeEnd != -1
-        rangeEnd = Math.max(@state.renderedRangeEnd, rangeEnd)
 
     # Final sanity check to prevent needless work
     return if rangeStart is @state.renderedRangeStart and
-              rangeEnd is @state.renderedRangeEnd and
+              rangeEnd is @state.renderedRangeEnd
-              scrollTop is @state.scrollTop
+
+    @updateRangeStateFiring = true
 
     @props.dataView.setRetainedRange
       start: rangeStart
       end: rangeEnd
 
     @setState
-      scrollTop: scrollTop
       renderedRangeStart: rangeStart
       renderedRangeEnd: rangeEnd
 
   render: =>
     innerStyles =
       height: @props.dataView.count() * @props.itemHeight
-      pointerEvents: if @state.scrollInProgress then 'none' else 'auto'
 
-    <ScrollRegion ref="container" onScroll={@updateScrollState} tabIndex="-1" className="list-container list-tabular" scrollTooltipComponent={@props.scrollTooltipComponent} >
+    <ScrollRegion ref="container" onScroll={@onScroll} tabIndex="-1" className="list-container list-tabular" scrollTooltipComponent={@props.scrollTooltipComponent} >
-      {@_headers()}
       <div className="list-rows" style={innerStyles}>
         {@_rows()}
       </div>
     </ScrollRegion>
 
-  _headers: =>
-    return [] unless @props.displayHeaders
-
-    headerColumns = @props.columns.map (column) ->
-      <div className="list-header list-column"
-           key={"header-#{column.name}"}
-           style={flex: column.flex}>
-        {column.name}
-      </div>
-
-    <div className="list-headers">
-      {headerColumns}
-    </div>
-
   _rows: =>
     rows = []
 

src/components/multiselect-list.cjsx

@@ -118,7 +118,7 @@ class MultiselectList extends React.Component
       <div className={className}>
         <ListTabular
           ref="list"
-          columns={@state.columns}
+          columns={@state.computedColumns}
           scrollTooltipComponent={@props.scrollTooltipComponent}
           dataView={@state.dataView}
           itemPropsProvider={@itemPropsProvider}
@@ -154,15 +154,8 @@ class MultiselectList extends React.Component
   _onShift: (delta, options = {}) =>
     @state.handler.onShift(delta, options)
 
-  # This onChange handler can be called many times back to back and setState
-  # sometimes triggers an immediate render. Ensure that we never render back-to-back,
-  # because rendering this view (even just to determine that there are no changes)
-  # is expensive.
   _onChange: =>
-    @_onChangeDebounced ?= _.debounce =>
+    @setState(@_getStateFromStores())
-      @setState(@_getStateFromStores())
-    , 1
-    @_onChangeDebounced()
 
   _visible: =>
     if WorkspaceStore.layoutMode() is "list"
@@ -184,22 +177,32 @@ class MultiselectList extends React.Component
 
   _getStateFromStores: (props) =>
     props ?= @props
+    state = @state ? {}
 
     view = props.dataStore?.view()
     return {} unless view
 
-    columns = [].concat(props.columns)
+    # Do we need to re-compute columns? Don't do this unless we really have to,
+    # it will cause a re-render of the entire ListTabular. To know whether our
+    # computed columns are still valid, we store the original columns in our state
+    # along with the computed ones.
+    if props.columns isnt state.columns
+      computedColumns = [].concat(props.columns)
+      if WorkspaceStore.layoutMode() is 'list'
+        computedColumns.splice(0, 0, @_getCheckmarkColumn())
+    else
+      computedColumns = state.computedColumns
 
     if WorkspaceStore.layoutMode() is 'list'
       handler = new MultiselectListInteractionHandler(view, props.collection)
-      columns.splice(0, 0, @_getCheckmarkColumn())
     else
       handler = new MultiselectSplitInteractionHandler(view, props.collection)
 
     dataView: view
-    columns: columns
     handler: handler
     ready: view.loaded()
+    columns: props.columns
+    computedColumns: computedColumns
     selectedIds: view.selection.ids()
     focusedId: FocusedContentStore.focusedId(props.collection)
     keyboardCursorId: FocusedContentStore.keyboardCursorId(props.collection)

src/components/scroll-region.cjsx

@@ -12,14 +12,16 @@ class ScrollRegion extends React.Component
 
   @propTypes:
     onScroll: React.PropTypes.func
+    onScrollEnd: React.PropTypes.func
     className: React.PropTypes.string
     scrollTooltipComponent: React.PropTypes.func
+    children: React.PropTypes.oneOfType([React.PropTypes.element, React.PropTypes.array])
 
   constructor: (@props) ->
     @state =
       totalHeight:0
       viewportHeight: 0
-      viewportOffset: 0
+      viewportScrollTop: 0
       dragging: false
       scrolling: false
 
@@ -31,14 +33,14 @@ class ScrollRegion extends React.Component
   componentDidMount: =>
     @_recomputeDimensions()
 
-  componentDidUpdate: =>
-    @_recomputeDimensions()
-
   componentWillUnmount: =>
     @_onHandleUp()
 
   shouldComponentUpdate: (newProps, newState) =>
-    not Utils.isEqualReact(newProps, @props) or not Utils.isEqualReact(newState, @state)
+    # Because this component renders @props.children, it needs to update
+    # on props.children changes. Unfortunately, computing isEqual on the
+    # @props.children tree extremely expensive. Just let React's algorithm do it's work.
+    true
 
   render: =>
     containerClasses =  "#{@props.className ? ''} " + classNames
@@ -50,7 +52,7 @@ class ScrollRegion extends React.Component
 
     tooltip = []
     if @props.scrollTooltipComponent
-      tooltip = <@props.scrollTooltipComponent viewportCenter={@state.viewportOffset + @state.viewportHeight / 2} totalHeight={@state.totalHeight} />
+      tooltip = <@props.scrollTooltipComponent viewportCenter={@state.viewportScrollTop + @state.viewportHeight / 2} totalHeight={@state.totalHeight} />
 
     <div className={containerClasses} {...otherProps}>
       <div className="scrollbar-track" style={@_scrollbarWrapStyles()} onMouseEnter={@_recomputeDimensions}>
@@ -61,7 +63,9 @@ class ScrollRegion extends React.Component
         </div>
       </div>
       <div className="scroll-region-content" onScroll={@_onScroll} ref="content">
-        {@props.children}
+        <div className="scroll-region-content-inner">
+          {@props.children}
+        </div>
       </div>
     </div>
 
@@ -81,7 +85,7 @@ class ScrollRegion extends React.Component
 
   _scrollbarHandleStyles: =>
     handleHeight = @_getHandleHeight()
-    handleTop = (@state.viewportOffset / (@state.totalHeight - @state.viewportHeight)) * (@state.trackHeight - handleHeight)
+    handleTop = (@state.viewportScrollTop / (@state.totalHeight - @state.viewportHeight)) * (@state.trackHeight - handleHeight)
 
     position:'relative'
     height: handleHeight
@@ -90,22 +94,31 @@ class ScrollRegion extends React.Component
   _getHandleHeight: =>
     Math.min(@state.totalHeight, Math.max(40, (@state.trackHeight / @state.totalHeight) * @state.trackHeight))
 
-  _recomputeDimensions: =>
+  _recomputeDimensions: ({avoidForcingLayout} = {}) =>
     return unless @refs.content
 
     contentNode = React.findDOMNode(@refs.content)
     trackNode = React.findDOMNode(@refs.track)
+    viewportScrollTop = contentNode.scrollTop
 
-    totalHeight = contentNode.scrollHeight
+    # While we're scrolling, calls to contentNode.scrollHeight / clientHeight
-    trackHeight = trackNode.clientHeight
+    # force the browser to immediately flush any DOM changes and compute the
-    viewportHeight = contentNode.clientHeight
+    # height of the node. This hurts performance and also kind of unnecessary,
-    viewportOffset = contentNode.scrollTop
+    # since it's unlikely these values will change while scrolling.
+    if avoidForcingLayout
+      totalHeight = @state.totalHeight ? contentNode.scrollHeight
+      trackHeight = @state.trackHeight ? contentNode.scrollHeight
+      viewportHeight = @state.viewportHeight ? contentNode.clientHeight
+    else
+      totalHeight = contentNode.scrollHeight
+      trackHeight = trackNode.clientHeight
+      viewportHeight = contentNode.clientHeight
 
     if @state.totalHeight != totalHeight or
        @state.trackHeight != trackHeight or
-       @state.viewportOffset != viewportOffset or
+       @state.viewportHeight != viewportHeight or
-       @state.viewportHeight != viewportHeight
+       @state.viewportScrollTop != viewportScrollTop
-      @setState({totalHeight, trackHeight, viewportOffset, viewportHeight})
+      @setState({totalHeight, trackHeight, viewportScrollTop, viewportHeight})
 
   _onHandleDown: (event) =>
     handleNode = React.findDOMNode(@refs.handle)
@@ -132,24 +145,24 @@ class ScrollRegion extends React.Component
     event.stopPropagation()
 
   _onScrollJump: (event) =>
+    @_trackOffset = React.findDOMNode(@refs.track).getBoundingClientRect().top
     @_mouseOffsetWithinHandle = @_getHandleHeight() / 2
     @_onHandleMove(event)
 
   _onScroll: (event) =>
-    if not @_requestedAnimationFrame
+    if not @state.scrolling
-      @_requestedAnimationFrame = true
+      @_recomputeDimensions()
-      window.requestAnimationFrame =>
+      @setState(scrolling: true)
-        @_requestedAnimationFrame = false
+    else
-        @_recomputeDimensions()
+      @_recomputeDimensions({avoidForcingLayout: true})
-        @props.onScroll?(event)
 
-        if not @state.scrolling
+    @props.onScroll?(event)
-          @setState(scrolling: true)
 
-        @_onStoppedScroll ?= _.debounce =>
+    @_onScrollEnd ?= _.debounce =>
-          @setState(scrolling: false)
+      @setState(scrolling: false)
-        , 250
+      @props.onScrollEnd?(event)
-        @_onStoppedScroll()
+    , 250
+    @_onScrollEnd()
 
 
 module.exports = ScrollRegion

src/error-reporter.js

@@ -163,8 +163,8 @@ module.exports = ErrorReporter = (function() {
     for (var ii = 1; ii < arguments.length; ii++) {
       args.push(arguments[ii]);
     }
-    if ((this.dev === true) && (showIt === true)) {
+    if ((this.inDevMode === true) && (showIt === true)) {
-      console.log.apply(this, args);
+      console.log.apply(console, args);
     }
     this.appendLog.apply(this, [args]);
   }

src/flux/attributes/attribute.coffee

@@ -22,6 +22,12 @@ class Attribute
     throw (new Error "this field cannot be queried against.") unless @queryable
     new Matcher(@, '=', val)
 
+  # Public: Returns a {Matcher} for objects `=` to the provided value.
+  in: (val) ->
+    throw (new Error "Attribute.in: you must pass an array of values.") unless val instanceof Array
+    throw (new Error "this field cannot be queried against.") unless @queryable
+    new Matcher(@, 'in', val)
+
   # Public: Returns a {Matcher} for objects `!=` to the provided value.
   not: (val) ->
     throw (new Error "this field cannot be queried against.") unless @queryable

src/flux/attributes/matcher.coffee

@@ -53,6 +53,7 @@ class Matcher
       when '=' then return value == @val
       when '<' then return value < @val
       when '>' then return value > @val
+      when 'in' then return value in @val
       when 'contains'
         # You can provide an ID or an object, and an array of IDs or an array of objects
         # Assumes that `value` is an array of items
@@ -84,6 +85,10 @@ class Matcher
       escaped = 1
     else if val is false
       escaped = 0
+    else if val instanceof Array
+      escapedVals = []
+      escapedVals.push("'#{v.replace(/'/g, '\\\'')}'") for v in val
+      escaped = "(#{escapedVals.join(',')})"
     else
       escaped = val
 

src/flux/models/message.coffee

@@ -132,6 +132,10 @@ class Message extends Model
   @naturalSortOrder: ->
     Message.attributes.date.ascending()
 
+  @additionalSQLiteConfig:
+    setup: ->
+      ['CREATE INDEX IF NOT EXISTS MessageListIndex ON Message(thread_id, date ASC)']
+
   constructor: ->
     super
     @subject ||= ""

src/flux/models/query.coffee

@@ -173,16 +173,20 @@ class ModelQuery
     if @_count
       return result[0][0] / 1
     else
-      objects = []
+      row = null
-      for i in [0..result.length-1] by 1
+      try
-        row = result[i]
+        objects = []
-        json = JSON.parse(row[0])
+        for i in [0..result.length-1] by 1
-        object = (new @_klass).fromJSON(json)
+          row = result[i]
-        for attr, j in @_includeJoinedData
+          json = JSON.parse(row[0])
-          value = row[j+1]
+          object = (new @_klass).fromJSON(json)
-          value = null if value is AttributeJoinedData.NullPlaceholder
+          for attr, j in @_includeJoinedData
-          object[attr.modelKey] = value
+            value = row[j+1]
-        objects.push(object)
+            value = null if value is AttributeJoinedData.NullPlaceholder
+            object[attr.modelKey] = value
+          objects.push(object)
+      catch jsonError
+        throw new Error("Query could not parse the database result. Query: #{@sql()}, Row Data: #{row[0]}, Error: #{jsonError.toString()}")
       return objects[0] if @_singular
       return objects
 

src/flux/models/thread.coffee

@@ -76,6 +76,10 @@ class Thread extends Model
 
   @getter 'unread', -> @isUnread()
 
+  @additionalSQLiteConfig:
+    setup: ->
+      ['CREATE INDEX IF NOT EXISTS ThreadListIndex ON Thread(last_message_timestamp DESC, namespace_id, id)']
+
   # Public: Returns an {Array} of {Tag} IDs
   #
   tagIds: ->
@@ -86,7 +90,10 @@ class Thread extends Model
   # * `id` A {String} {Tag} ID
   #
   hasTagId: (id) ->
-    @tagIds().indexOf(id) != -1
+    return false unless id and @tags
+    for tag in @tags
+      return true if tag.id is id
+    return false
 
   # Public: Returns a {Boolean}, true if the thread is unread.
   isUnread: ->

src/flux/stores/database-store.coffee

@@ -43,7 +43,7 @@ class DatabaseProxy
         duration: duration
         result_length: result?.length
 
-      console.debug(printToConsole, "DatabaseStore: #{query}", metadata)
+      console.debug(printToConsole, "DatabaseStore: (#{duration}) #{query}", metadata)
       if duration > 300
         atom.errorReporter.shipLogs("Poor Query Performance")
 
@@ -320,6 +320,12 @@ class DatabaseStore
         if model[attr.modelKey]?
           tx.execute("REPLACE INTO `#{attr.modelTable}` (`id`, `value`) VALUES (?, ?)", [model.id, model[attr.modelKey]])
 
+    # For each model, execute any other code the model wants to run.
+    # This allows model classes to do things like update a full-text table
+    # that holds a composite of several fields
+    if klass.additionalSQLiteConfig?.writeModel?
+      for model in models
+        klass.additionalSQLiteConfig.writeModel(tx, model)
 
   deleteModel: (tx, model) =>
     klass = model.constructor
@@ -343,6 +349,12 @@ class DatabaseStore
     joinedDataAttributes.forEach (attr) ->
       tx.execute("DELETE FROM `#{attr.modelTable}` WHERE `id` = ?", [model.id])
 
+    # Execute any other code the model wants to run.
+    # This allows model classes to do things like update a full-text table
+    # that holds a composite of several fields, or update entirely
+    # separate database systems
+    klass.additionalSQLiteConfig?.deleteModel?(tx, model)
+
   # Public: Asynchronously writes `model` to the cache and triggers a change event.
   #
   # - `model` A {Model} to write to the database.
@@ -553,9 +565,6 @@ class DatabaseStore
     columns = ['id TEXT PRIMARY KEY', 'data BLOB']
     columnAttributes.forEach (attr) ->
       columns.push(attr.columnSQL())
-      # TODO: These indexes are not effective because SQLite only uses one index-per-table
-      # and there will almost always be an additional `where namespaceId =` clause.
-      queries.push("CREATE INDEX IF NOT EXISTS `#{klass.name}_#{attr.jsonKey}` ON `#{klass.name}` (`#{attr.jsonKey}`)")
 
     columnsSQL = columns.join(',')
     queries.unshift("CREATE TABLE IF NOT EXISTS `#{klass.name}` (#{columnsSQL})")
@@ -576,6 +585,8 @@ class DatabaseStore
     joinedDataAttributes.forEach (attribute) ->
       queries.push("CREATE TABLE IF NOT EXISTS `#{attribute.modelTable}` (id TEXT PRIMARY KEY, `value` TEXT)")
 
+    if klass.additionalSQLiteConfig?.setup?
+      queries = queries.concat(klass.additionalSQLiteConfig.setup())
     queries
 
 

src/flux/stores/database-view.coffee

@@ -31,7 +31,7 @@ verbose = true
 #
 class DatabaseView extends ModelView
 
-  constructor: (@klass, config = {}, @_itemMetadataProvider) ->
+  constructor: (@klass, config = {}, @_metadataProvider) ->
     super
     @_pageSize = 100
     @_matchers = config.matchers ? []
@@ -49,11 +49,11 @@ class DatabaseView extends ModelView
       arguments[0] = "DatabaseView (#{@klass.name}): "+arguments[0]
     console.log(arguments...)
 
-  itemMetadataProvider: ->
+  metadataProvider: ->
-    @_itemMetadataProvider
+    @_metadataProvider
 
-  setItemMetadataProvider: (fn) ->
+  setMetadataProvider: (fn) ->
-    @_itemMetadataProvider = fn
+    @_metadataProvider = fn
     @_pages = {}
     @invalidate()
 
@@ -314,16 +314,16 @@ class DatabaseView extends ModelView
       @retrievePage(idx)
       return
 
-    metadataPromises = {}
+    idsMissingMetadata = []
     for item in items
-      if metadataPromises[item.id]
+      if not page.metadata[item.id]
-        @log("Request for threads returned the same thread id (#{item.id}) multiple times.")
+        idsMissingMetadata.push(item.id)
 
-      metadataPromises[item.id] ?= page.metadata[item.id]
+    metadataPromise = Promise.resolve({})
-      if @_itemMetadataProvider
+    if idsMissingMetadata.length > 0 and @_metadataProvider
-        metadataPromises[item.id] ?= @_itemMetadataProvider(item)
+      metadataPromise = @_metadataProvider(idsMissingMetadata)
 
-    Promise.props(metadataPromises).then (results) =>
+    metadataPromise.then (results) =>
       # If we've started reloading since we made our query, don't do any more work
       if page.lastTouchTime >= touchTime
         @log("Metadata version #{touchTime} fetched, but out of date (current is #{page.lastTouchTime})")
@@ -332,8 +332,8 @@ class DatabaseView extends ModelView
       for item, idx in items
         if Object.isFrozen(item)
           item = items[idx] = new @klass(item)
-        item.metadata = results[item.id]
+        metadata = results[item.id] ? page.metadata[item.id]
-        page.metadata[item.id] = results[item.id]
+        item.metadata = page.metadata[item.id] = metadata
 
         # Prevent anything from mutating these objects or their nested objects.
         # Accidentally modifying items somewhere downstream (in a component)

static/components/list-tabular.less

@@ -160,12 +160,6 @@
     }
   }
 
-  .list-rows {
-    overflow: auto;
-    // Add back when when we re-implement list-headers
-    // padding-top: @font-size-base * 2; /* height of list-headers*/
-  }
-
   .list-column {
     // The width is set by React.
     display: inline-block;

static/components/scroll-region.less

@@ -51,6 +51,9 @@
     width: 100%;
     overflow-y: scroll;
   }
+  .scroll-region-content-inner {
+    transform:translate3d(0,0,0);
+  }
 
   .scrollbar-track {
     opacity: 0;
@@ -87,6 +90,11 @@
   }
 }
 .scroll-region.scrolling {
+
+  .scroll-region-content-inner {
+    pointer-events: none;
+  }
+
   .scrollbar-track {
     opacity: 1;
     transition-delay: 0s;