🐛 Remove Gutter from ViewRegistry

Closes #7306

We started noticing that when a `TextEditor` pane got split, the same view for
`Gutter` was being shared amongst several models, thereby making the same DOM
element accessible simultaneously by more than one object. This made us
experience *orphaned line numbers*, caused by two instances of
`LineNumberGutterComponent` mutating `.line-numbers` at the same time.

This is a typical race condition which I would normally address by understanding
and possibly locking the correct order in which operations should happen.
However, I believe in this situation we shouldn’t actually care about ordering
at all, since I think views should be kept “local”, thus avoiding to expose them
to the world and/or reusing them across other views (either accidentally as in
this case or on purpose).
This commit is contained in:
Antonio Scandurra
2015-06-17 12:24:35 +02:00
parent 62dc15a697
commit b0d93accf7
5 changed files with 25 additions and 30 deletions

View File

@@ -16,9 +16,6 @@ describe "CustomGutterComponent", ->
decorationsWrapperNode = gutterComponent.getDomNode().children.item(0)
expect(decorationsWrapperNode.classList.contains('custom-decorations')).toBe true
it "makes its view accessible from the view registry", ->
expect(gutterComponent.getDomNode()).toBe atom.views.getView(gutter)
it "hides its DOM node when ::hideNode is called, and shows its DOM node when ::showNode is called", ->
gutterComponent.hideNode()
expect(gutterComponent.getDomNode().style.display).toBe 'none'

View File

@@ -107,22 +107,24 @@ describe "GutterContainerComponent", ->
gutterContainerComponent.updateSync(testState)
expect(gutterContainerComponent.getDomNode().children.length).toBe 2
expectedCustomGutterNode = gutterContainerComponent.getDomNode().children.item(0)
expect(expectedCustomGutterNode).toBe atom.views.getView(customGutter1)
expectedLineNumbersNode = gutterContainerComponent.getDomNode().children.item(1)
expect(expectedLineNumbersNode).toBe atom.views.getView(lineNumberGutter)
initialCustomGutterNode1 = gutterContainerComponent.getDomNode().children.item(0)
initialLineNumbersNode = gutterContainerComponent.getDomNode().children.item(1)
# Add a gutter.
customGutter2 = new Gutter(mockGutterContainer, {name: 'custom2', priority: -10})
testState = buildTestState([customGutter1, customGutter2, lineNumberGutter])
gutterContainerComponent.updateSync(testState)
expect(gutterContainerComponent.getDomNode().children.length).toBe 3
expectedCustomGutterNode1 = gutterContainerComponent.getDomNode().children.item(0)
expect(expectedCustomGutterNode1).toBe atom.views.getView(customGutter1)
expectedCustomGutterNode2 = gutterContainerComponent.getDomNode().children.item(1)
expect(expectedCustomGutterNode2).toBe atom.views.getView(customGutter2)
expectedLineNumbersNode = gutterContainerComponent.getDomNode().children.item(2)
expect(expectedLineNumbersNode).toBe atom.views.getView(lineNumberGutter)
unchangedCustomGutterNode1 = gutterContainerComponent.getDomNode().children.item(0)
insertedCustomGutterNode2 = gutterContainerComponent.getDomNode().children.item(1)
repositionedLineNumbersNode = gutterContainerComponent.getDomNode().children.item(2)
expect(initialCustomGutterNode1).toBe(unchangedCustomGutterNode1)
expect(initialLineNumbersNode).toBe(repositionedLineNumbersNode)
expect(insertedCustomGutterNode2).toBeDefined()
# Hide one gutter, reposition one gutter, remove one gutter; and add a new gutter.
customGutter2.hide()
@@ -130,10 +132,12 @@ describe "GutterContainerComponent", ->
testState = buildTestState([customGutter2, customGutter1, customGutter3])
gutterContainerComponent.updateSync(testState)
expect(gutterContainerComponent.getDomNode().children.length).toBe 3
expectedCustomGutterNode2 = gutterContainerComponent.getDomNode().children.item(0)
expect(expectedCustomGutterNode2).toBe atom.views.getView(customGutter2)
expect(expectedCustomGutterNode2.style.display).toBe 'none'
expectedCustomGutterNode1 = gutterContainerComponent.getDomNode().children.item(1)
expect(expectedCustomGutterNode1).toBe atom.views.getView(customGutter1)
expectedCustomGutterNode3 = gutterContainerComponent.getDomNode().children.item(2)
expect(expectedCustomGutterNode3).toBe atom.views.getView(customGutter3)
repositionedCustomGutterNode2 = gutterContainerComponent.getDomNode().children.item(0)
repositionedCustomGutterNode1 = gutterContainerComponent.getDomNode().children.item(1)
insertedCustomGutterNode3 = gutterContainerComponent.getDomNode().children.item(2)
expect(initialCustomGutterNode1).toBe(repositionedCustomGutterNode1)
expect(repositionedCustomGutterNode2).toBe(insertedCustomGutterNode2)
expect(repositionedCustomGutterNode2.style.display).toBe('none')
expect(insertedCustomGutterNode3).toBeDefined()

View File

@@ -1,4 +1,4 @@
{setDimensionsAndBackground} = require './gutter-component-helpers'
{createGutterView, setDimensionsAndBackground} = require './gutter-component-helpers'
# This class represents a gutter other than the 'line-numbers' gutter.
# The contents of this gutter may be specified by Decorations.
@@ -11,10 +11,8 @@ class CustomGutterComponent
@decorationItemsById = {}
@visible = true
@domNode = atom.views.getView(@gutter)
@domNode = createGutterView(@gutter)
@decorationsNode = @domNode.firstChild
# Clear the contents in case the domNode is being reused.
@decorationsNode.innerHTML = ''
getDomNode: ->
@domNode

View File

@@ -1,5 +1,5 @@
_ = require 'underscore-plus'
{setDimensionsAndBackground} = require './gutter-component-helpers'
{createGutterView, setDimensionsAndBackground} = require './gutter-component-helpers'
WrapperDiv = document.createElement('div')
@@ -11,9 +11,8 @@ class LineNumberGutterComponent
@lineNumberNodesById = {}
@visible = true
@domNode = atom.views.getView(@gutter)
@domNode = createGutterView(@gutter)
@lineNumbersNode = @domNode.firstChild
@lineNumbersNode.innerHTML = ''
@domNode.addEventListener 'click', @onClick
@domNode.addEventListener 'mousedown', @onMouseDown

View File

@@ -2,8 +2,6 @@
Grim = require 'grim'
{Emitter, CompositeDisposable} = require 'event-kit'
Serializable = require 'serializable'
{createGutterView} = require './gutter-component-helpers'
Gutter = require './gutter'
Model = require './model'
Pane = require './pane'
PaneElement = require './pane-element'
@@ -62,7 +60,6 @@ class PaneContainer extends Model
new PaneElement().initialize(model)
atom.views.addViewProvider TextEditor, (model) ->
new TextEditorElement().initialize(model)
atom.views.addViewProvider(Gutter, createGutterView)
onDidChangeRoot: (fn) ->
@emitter.on 'did-change-root', fn