Keep model's logical scroll position up to date

* Remove scrollRow and scrollColumn properties from the presenter
* Assign presenter's scrollTop and scrollLeft based on model's first visible
  screen row and column, once the presenter has the required measurements.

spec/text-editor-presenter-spec.coffee

@@ -35,11 +35,9 @@ describe "TextEditorPresenter", ->
       presenter
 
     buildPresenter = (params={}) ->
-      presenter = buildPresenterWithoutMeasurements(_.defaults(params,
-        scrollTop: 0
-        scrollLeft: 0
-      ))
-
+      presenter = buildPresenterWithoutMeasurements(params)
+      presenter.setScrollTop(params.scrollTop) if params.scrollTop?
+      presenter.setScrollLeft(params.scrollLeft) if params.scrollLeft?
       presenter.setExplicitHeight(params.explicitHeight ? 130)
       presenter.setWindowSize(params.windowWidth ? 500, params.windowHeight ? 130)
       presenter.setBoundingClientRect(params.boundingClientRect ? {
@@ -175,10 +173,8 @@ describe "TextEditorPresenter", ->
         presenter.setExplicitHeight(25)
         expect(stateFn(presenter).tiles).toEqual({})
 
+        # Sets scroll row from model's logical position
         presenter.setLineHeight(10)
-        expect(stateFn(presenter).tiles).toEqual({})
-
-        presenter.setScrollTop(0)
         expect(stateFn(presenter).tiles).not.toEqual({})
 
       it "updates when ::scrollTop changes", ->
@@ -621,6 +617,8 @@ describe "TextEditorPresenter", ->
       describe ".scrollingVertically", ->
         it "is true for ::stoppedScrollingDelay milliseconds following a changes to ::scrollTop", ->
           presenter = buildPresenter(scrollTop: 10, stoppedScrollingDelay: 200, explicitHeight: 100)
+          expect(presenter.getState().content.scrollingVertically).toBe true
+          advanceClock(300)
           expect(presenter.getState().content.scrollingVertically).toBe false
           expectStateUpdate presenter, -> presenter.setScrollTop(0)
           expect(presenter.getState().content.scrollingVertically).toBe true
@@ -763,7 +761,8 @@ describe "TextEditorPresenter", ->
           expect(presenter.getState().content.scrollTop).toBe(10)
 
         it "corresponds to the passed logical coordinates when building the presenter", ->
-          presenter = buildPresenter(scrollRow: 4, lineHeight: 10, explicitHeight: 20)
+          editor.setFirstVisibleScreenRow(4)
+          presenter = buildPresenter(lineHeight: 10, explicitHeight: 20)
           expect(presenter.getState().content.scrollTop).toBe(40)
 
         it "tracks the value of ::scrollTop", ->
@@ -777,11 +776,11 @@ describe "TextEditorPresenter", ->
 
           expectStateUpdate presenter, -> presenter.setScrollTop(50)
           presenter.getState() # commits scroll position
-          expect(editor.getScrollRow()).toBe(5)
+          expect(editor.getFirstVisibleScreenRow()).toBe 5
 
           expectStateUpdate presenter, -> presenter.setScrollTop(57)
           presenter.getState() # commits scroll position
-          expect(editor.getScrollRow()).toBe(6)
+          expect(editor.getFirstVisibleScreenRow()).toBe 6
 
         it "reassigns the scrollTop if it exceeds the max possible value after lines are removed", ->
           presenter = buildPresenter(scrollTop: 80, lineHeight: 10, explicitHeight: 50, horizontalScrollbarHeight: 0)
@@ -890,7 +889,8 @@ describe "TextEditorPresenter", ->
           expect(presenter.getState().content.scrollLeft).toBe(50)
 
         it "corresponds to the passed logical coordinates when building the presenter", ->
-          presenter = buildPresenter(scrollColumn: 3, lineHeight: 10, baseCharacterWidth: 10, verticalScrollbarWidth: 10, contentFrameWidth: 500)
+          editor.setFirstVisibleScreenColumn(3)
+          presenter = buildPresenter(lineHeight: 10, baseCharacterWidth: 10, verticalScrollbarWidth: 10, contentFrameWidth: 500)
           expect(presenter.getState().content.scrollLeft).toBe(30)
 
         it "tracks the value of ::scrollLeft", ->
@@ -904,11 +904,11 @@ describe "TextEditorPresenter", ->
 
           expectStateUpdate presenter, -> presenter.setScrollLeft(50)
           presenter.getState() # commits scroll position
-          expect(editor.getScrollColumn()).toBe(5)
+          expect(editor.getFirstVisibleScreenColumn()).toBe 5
 
           expectStateUpdate presenter, -> presenter.setScrollLeft(57)
           presenter.getState() # commits scroll position
-          expect(editor.getScrollColumn()).toBe(6)
+          expect(editor.getFirstVisibleScreenColumn()).toBe 6
 
         it "is always rounded to the nearest integer", ->
           presenter = buildPresenter(scrollLeft: 10, lineHeight: 10, baseCharacterWidth: 10, verticalScrollbarWidth: 10, contentFrameWidth: 500)

src/text-editor-component.coffee

@@ -50,10 +50,6 @@ class TextEditorComponent
 
     @presenter = new TextEditorPresenter
       model: @editor
-      scrollTop: 0
-      scrollLeft: 0
-      scrollRow: @editor.getScrollRow()
-      scrollColumn: @editor.getScrollColumn()
       tileSize: tileSize
       cursorBlinkPeriod: @cursorBlinkPeriod
       cursorBlinkResumeDelay: @cursorBlinkResumeDelay

src/text-editor-presenter.coffee

@@ -15,7 +15,7 @@ class TextEditorPresenter
   constructor: (params) ->
     {@model, @config} = params
     {@cursorBlinkPeriod, @cursorBlinkResumeDelay, @stoppedScrollingDelay, @tileSize} = params
-    {@contentFrameWidth, @scrollTop, @scrollLeft, @scrollColumn, @scrollRow} = params
+    {@contentFrameWidth} = params
 
     @gutterWidth = 0
     @tileSize ?= 6
@@ -72,7 +72,6 @@ class TextEditorPresenter
     @updateVerticalDimensions()
     @updateScrollbarDimensions()
 
-    @restoreScrollPosition()
     @commitPendingLogicalScrollTopPosition()
     @commitPendingScrollTopPosition()
 
@@ -777,8 +776,7 @@ class TextEditorPresenter
     if scrollTop isnt @realScrollTop and not Number.isNaN(scrollTop)
       @realScrollTop = scrollTop
       @scrollTop = Math.round(scrollTop)
-      @scrollRow = Math.round(@scrollTop / @lineHeight)
-      @model.setScrollRow(@scrollRow)
+      @model.setFirstVisibleScreenRow(Math.round(@scrollTop / @lineHeight))
 
       @updateStartRow()
       @updateEndRow()
@@ -794,8 +792,7 @@ class TextEditorPresenter
     if scrollLeft isnt @realScrollLeft and not Number.isNaN(scrollLeft)
       @realScrollLeft = scrollLeft
       @scrollLeft = Math.round(scrollLeft)
-      @scrollColumn = Math.round(@scrollLeft / @baseCharacterWidth)
-      @model.setScrollColumn(@scrollColumn)
+      @model.setFirstVisibleScreenColumn(Math.round(@scrollLeft / @baseCharacterWidth))
 
       @emitter.emit 'did-change-scroll-left', @scrollLeft
 
@@ -1106,6 +1103,8 @@ class TextEditorPresenter
   setLineHeight: (lineHeight) ->
     unless @lineHeight is lineHeight
       @lineHeight = lineHeight
+      unless @scrollTop?
+        @updateScrollTop(@model.getFirstVisibleScreenRow() * lineHeight)
       @model.setLineHeightInPixels(lineHeight)
       @shouldUpdateHeightState = true
       @shouldUpdateHorizontalScrollState = true
@@ -1133,6 +1132,8 @@ class TextEditorPresenter
       @halfWidthCharWidth = halfWidthCharWidth
       @koreanCharWidth = koreanCharWidth
       @model.setDefaultCharWidth(baseCharacterWidth, doubleWidthCharWidth, halfWidthCharWidth, koreanCharWidth)
+      unless @scrollLeft?
+        @updateScrollLeft(@model.getFirstVisibleScreenColumn() * baseCharacterWidth)
       @characterWidthsChanged()
 
   characterWidthsChanged: ->
@@ -1617,14 +1618,6 @@ class TextEditorPresenter
       @updateScrollTop(@pendingScrollTop)
       @pendingScrollTop = null
 
-  restoreScrollPosition: ->
-    return if @hasRestoredScrollPosition or not @hasPixelPositionRequirements()
-
-    @setScrollTop(@scrollRow * @lineHeight) if @scrollRow?
-    @setScrollLeft(@scrollColumn * @baseCharacterWidth) if @scrollColumn?
-
-    @hasRestoredScrollPosition = true
-
   clearPendingScrollPosition: ->
     @pendingScrollLogicalPosition = null
     @pendingScrollTop = null

src/text-editor.coffee

@@ -87,7 +87,7 @@ class TextEditor extends Model
     super
 
     {
-      @softTabs, @scrollRow, @scrollColumn, initialLine, initialColumn, tabLength,
+      @softTabs, @firstVisibleScreenRow, @firstVisibleScreenColumn, initialLine, initialColumn, tabLength,
       softWrapped, @displayBuffer, buffer, suppressCursorCreation, @mini, @placeholderText,
       lineNumberGutterVisible, largeFileMode, @config, @notificationManager, @packageManager,
       @clipboard, @viewRegistry, @grammarRegistry, @project, @assert, @applicationDelegate
@@ -102,6 +102,8 @@ class TextEditor extends Model
     throw new Error("Must pass a project parameter when constructing TextEditors") unless @project?
     throw new Error("Must pass an assert parameter when constructing TextEditors") unless @assert?
 
+    @firstVisibleScreenRow ?= 0
+    @firstVisibleScreenColumn ?= 0
     @emitter = new Emitter
     @disposables = new CompositeDisposable
     @cursors = []
@@ -141,8 +143,8 @@ class TextEditor extends Model
     deserializer: 'TextEditor'
     id: @id
     softTabs: @softTabs
-    scrollRow: @getScrollRow()
-    scrollColumn: @getScrollColumn()
+    firstVisibleScreenRow: @getFirstVisibleScreenRow()
+    firstVisibleScreenColumn: @getFirstVisibleScreenColumn()
     displayBuffer: @displayBuffer.serialize()
 
   subscribeToBuffer: ->
@@ -3086,11 +3088,12 @@ class TextEditor extends Model
     Grim.deprecate("This is now a view method. Call TextEditorElement::getWidth instead.")
     @displayBuffer.getWidth()
 
-  getScrollRow: -> @scrollRow
-  setScrollRow: (@scrollRow) ->
+  setFirstVisibleScreenRow: (@firstVisibleScreenRow) ->
+  getFirstVisibleScreenRow: -> @firstVisibleScreenRow
+
+  setFirstVisibleScreenColumn: (@firstVisibleScreenColumn) ->
+  getFirstVisibleScreenColumn: -> @firstVisibleScreenColumn
 
-  getScrollColumn: -> @scrollColumn
-  setScrollColumn: (@scrollColumn) ->
 
   getScrollTop: ->
     Grim.deprecate("This is now a view method. Call TextEditorElement::getScrollTop instead.")