Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

618-Bloc-Main-Thread-should-be-stopped-and-restarted-in-shutdownstartup #619

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 0 additions & 89 deletions src/Bloc-Tests/BlSpaceTest.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -7,92 +7,3 @@ Class {
],
#category : #'Bloc-Tests-Space'
}

{ #category : #initialization }
BlSpaceTest >> dirtyAreas [
^ space dirtyAreas asArray collect: #asRectangle
]

{ #category : #initialization }
BlSpaceTest >> setUp [
super setUp.

space := BlSpace new.
host := BlMockedHost new.

space host: host.
host createHostSpaceFor: space.
]

{ #category : #initialization }
BlSpaceTest >> tearDown [
super tearDown.

host destroyHostSpaceFor: space.
space host: nil.
host := nil.
space := nil
]

{ #category : #tests }
BlSpaceTest >> testEnqueueEvent [

self skip.
space pulse.
self assert: space pulseRequested not.

space hostSpace enqueueEvent:
(BlMouseMoveEvent new
position: 20 @ 20;
yourself).
self assert: space pulseRequested.

space pulse.
self assert: space pulseRequested not
]

{ #category : #tests }
BlSpaceTest >> testInitialization [
self skip.
"Host space must not be nil"
self assert: space hostSpace isNotNil.

"Space must request pulse when just created"
self assert: space pulseRequested.

"Space must have its whole area being dirty"
self assert: self dirtyAreas equals: { 0@0 extent: space extent }
]

{ #category : #tests }
BlSpaceTest >> testRender [
self skip.
self assert: self dirtyAreas equals: { 0@0 extent: space extent }.
self assert: space hostSpace renderCount equals: 0.
self assert: space pulseRequested.

space pulse.
self assert: self dirtyAreas equals: {}.
self assert: space hostSpace renderCount equals: 1.
self assert: space pulseRequested not.

space pulse.
self assert: self dirtyAreas equals: {}.
self assert: space hostSpace renderCount equals: 1.
self assert: space pulseRequested not.

space requestNextPulse.
self assert: space pulseRequested.
space pulse.
self assert: self dirtyAreas equals: {}.
self assert: space hostSpace renderCount equals: 1.
self assert: space pulseRequested not.

space invalidRect: (BlBounds origin: [email protected] extent: [email protected]) from: space root.
self assert: space pulseRequested.
self assert: self dirtyAreas equals: { 50@70 extent: 300@200 }.
space pulse.
self assert: self dirtyAreas equals: {}.
self assert: space hostSpace renderCount equals: 2.
self assert: space pulseRequested not.
]
50 changes: 50 additions & 0 deletions src/Bloc-Tests/BlSpaceVisibilityTest.class.st
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
Class {
#name : #BlSpaceVisibilityTest,
#superclass : #BlParameterizedHostTest,
#category : #'Bloc-Tests-Space'
}

{ #category : #tests }
BlSpaceVisibilityTest >> testHidingAnOpenSpaceHidesTheWindow [

| aSpace |

Smalltalk vm operatingSystemName = 'unix' ifTrue: [ ^ self skip ].

aSpace := self newTestingSpace.

aSpace show.
self waitTestingOpenedSpaces.

self assert: aSpace hostSpace isVisible.

aSpace hide.

self deny: aSpace hostSpace isVisible.

]

{ #category : #tests }
BlSpaceVisibilityTest >> testHidingAndShowingAnOpenSpaceShowsTheWindow [

| aSpace |

Smalltalk vm operatingSystemName = 'unix' ifTrue: [ ^ self skip ].

aSpace := self newTestingSpace.

aSpace show.
self waitTestingOpenedSpaces.

self assert: aSpace hostSpace isVisible.

aSpace hide.

self deny: aSpace hostSpace isVisible.

aSpace show.
self waitTestingOpenedSpaces.

self assert: aSpace hostSpace isVisible.

]
81 changes: 81 additions & 0 deletions src/Bloc-Tests/BlStartupShutdownTest.class.st
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
Class {
#name : #BlStartupShutdownTest,
#superclass : #BlParameterizedHostTest,
#category : #'Bloc-Tests-Space'
}

{ #category : #running }
BlStartupShutdownTest >> runCaseManaged [

^ self runCase
]

{ #category : #tests }
BlStartupShutdownTest >> testShutdownAndStartupKeepsSpacesInSamePosition [

| aSpace |

hostClass = BlMorphicWindowHost ifTrue: [ ^ self skip ].
Smalltalk vm operatingSystemName = 'unix' ifTrue: [ ^ self skip ].

aSpace := self newTestingSpace.

aSpace show.
self waitTestingOpenedSpaces.
aSpace position: 100@100.
aSpace extent: 200@200.

self assert: aSpace position equals: 100@100.
self assert: aSpace extent equals: 200@200.

aSpace universe stopUniverse.
aSpace universe startUniverse.

self assert: aSpace position equals: 100@100.
self assert: aSpace extent equals: 200@200.

]

{ #category : #tests }
BlStartupShutdownTest >> testShutdownAndStartupKeepsSpacesOpen [

| aSpace |

hostClass = BlMorphicWindowHost ifTrue: [ ^ self skip ].
Smalltalk vm operatingSystemName = 'unix' ifTrue: [ ^ self skip ].

aSpace := self newTestingSpace.

aSpace show.
self waitTestingOpenedSpaces.
self assert: aSpace isVisible.

aSpace universe stopUniverse.
aSpace universe startUniverse.

self assert: aSpace isVisible.

]

{ #category : #tests }
BlStartupShutdownTest >> testShutdownAndStartupKeepsSpacesOpenWithInvalidation [

| aSpace |

hostClass = BlMorphicWindowHost ifTrue: [ ^ self skip ].
Smalltalk vm operatingSystemName = 'unix' ifTrue: [ ^ self skip ].

aSpace := self newTestingSpace.

aSpace show.
self waitTestingOpenedSpaces.
self assert: aSpace isVisible.

aSpace universe stopUniverse.
aSpace hostSpace invalidate.

aSpace universe startUniverse.

self assert: aSpace isVisible.

]
6 changes: 6 additions & 0 deletions src/Bloc/BlHost.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ BlHost >> offscreenMeasureTextParagraph: aBlTextParagraph [
BlHostRenderer preferableClass offscreenMeasureTextParagraph: aBlTextParagraph
]

{ #category : #'host - testing' }
BlHost >> runOneCycle [

"The host can do something while waiting for the pulse to happen. Useful for Morphic when the test run in the UI thread"
]

{ #category : #'api - lifecycle' }
BlHost >> start [

Expand Down
28 changes: 24 additions & 4 deletions src/Bloc/BlParallelUniverse.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,21 @@ BlParallelUniverse class >> forHost: aHostClass [
BlParallelUniverse class >> initialize [
Universes := #().
UniversesMutex := Mutex new.
UniqueIdGenerator := BlUniqueIdGenerator new
UniqueIdGenerator := BlUniqueIdGenerator new.

SessionManager default registerGuiClassNamed: self name
]

{ #category : #'system startup' }
BlParallelUniverse class >> shutDown [

self all do: [ :anUniverse | anUniverse stopUniverse ]
]

{ #category : #'system startup' }
BlParallelUniverse class >> startUp [

self all do: [ :anUniverse | anUniverse startUniverse ]
]

{ #category : #'api - spaces' }
Expand Down Expand Up @@ -297,6 +311,7 @@ BlParallelUniverse >> hasUIProcess [

{ #category : #accessing }
BlParallelUniverse >> hostClass [

^ hostClass
]

Expand Down Expand Up @@ -354,7 +369,10 @@ BlParallelUniverse >> openSpace: aSpace [
{ #category : #'private - spaces' }
BlParallelUniverse >> openSpaceSynchronously: aSpace [

aSpace isOpened ifTrue: [ ^ self ].
aSpace isOpened ifTrue: [
"If it has been already opened, just unhide it"
aSpace hostSpace show.
^ self ].
Comment on lines -357 to +375
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to review this change together with #636. Maybe what needed to happen in this case is to attach the space to the universe as if it was a host switch.


self
assert: [ aSpace hasHostSpace not ]
Expand Down Expand Up @@ -450,15 +468,17 @@ BlParallelUniverse >> startUniverse [
"A universe must not be running here.
I am called outside of the UI loop (there is no UI loop yet)"

self hostClass start
hostClass start.
spaceManager start
]

{ #category : #'private - lifecycle' }
BlParallelUniverse >> stopUniverse [
"A universe must be running here.
I am called from the UI loop"

self hostClass stop
hostClass stop.
spaceManager stop
]

{ #category : #'deferred message' }
Expand Down
2 changes: 1 addition & 1 deletion src/Bloc/BlRealTime.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ BlRealTime >> every: aDuration while: aWhileBlock do: aDoBlock [
aStartTime := self now.
aCount := 0.

aWhileBlock whileTrue: [
[aWhileBlock value] whileTrue: [
| anExpectedTime aCurrentTime aWaitingTime |
aDoBlock value.

Expand Down
23 changes: 21 additions & 2 deletions src/Bloc/BlSpace.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ Class {
'reference',
'iconStencil',
'rootElement',
'userData'
'userData',
'previousVisibleStatus'
],
#classVars : [
'UniqueIdGenerator'
Expand Down Expand Up @@ -139,7 +140,10 @@ BlSpace class >> pulseUntilEmptyTaskQueue: aSpace timeout: aDuration [
aSpace universe hasDeferredActions or: [
(aSpace taskQueue isEmpty or: [
aSpace time now >= deadline ]) not ] ]
do: [ aSpace pulse ].
do: [
"We delegate to the host class also to run a cycle, it is needed when using morphic host, as this can run in the same UI thread blocking everything."
aSpace host runOneCycle.
aSpace pulse ].

aSpace pulse.

Expand Down Expand Up @@ -864,6 +868,15 @@ BlSpace >> ensureSession [
self onSessionChanged: Smalltalk session
]

{ #category : #'private - display' }
BlSpace >> ensureWindowOpen [

hostSpace ifNotNil: [
hostSpace isValid ifFalse: [
host createHostSpaceFor: self.
previousVisibleStatus ifTrue: [ hostSpace open ] ] ]
]

{ #category : #'event management accessing' }
BlSpace >> eventDispatcher [
^ eventDispatcher
Expand Down Expand Up @@ -1480,6 +1493,12 @@ BlSpace >> pulseRequested [
^ nextPulseRequested
]

{ #category : #'startup - shutdown' }
BlSpace >> rememberVisibleStatus [

previousVisibleStatus := self isVisible
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust in your solution, but I didn't realize yet why isVisible is not enough, and we need a new flag

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we need to store the flag. Once the real window is destroyed / do not exist in the starting image, the window is always not visible. We need to know how it was before saving the image. Thanks for the review. I tried to avoid it but the space only delegates this behaviour to the real window.

]

{ #category : #pulse }
BlSpace >> render [
"Render this space in my host window if it is assigned, otherwise do nothing"
Expand Down
12 changes: 12 additions & 0 deletions src/Bloc/BlSpaceManager.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,15 @@ BlSpaceManager >> spaces [

^ spaces
]

{ #category : #'startup - shutdown' }
BlSpaceManager >> start [

spaces do: [ :aSpace | aSpace ensureWindowOpen ]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class has the only sends of ensureWindowOpen and rememberVisibleStatus. Maybe previousVisibleStatus flag fits better here in BlSpaceManager instead of BlSpace. It is here where this previousVisibleStatus is stored and restored.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The visible status is of the window (aka the space) if we put the flag in the space manager we need a collection of flags (one per each space).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. We need to store the state of the open windows if we want to reopen in the same place.

]

{ #category : #'startup - shutdown' }
BlSpaceManager >> stop [

spaces do: [ :anSpace | anSpace rememberVisibleStatus ]
]
7 changes: 7 additions & 0 deletions src/BlocHost-Morphic/BlMorphicSteppingHost.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ BlMorphicSteppingHost class >> uiProcessDo: aBlock [
UIManager default uiProcess ifNotNil: aBlock
]

{ #category : #'host - testing' }
BlMorphicSteppingHost >> runOneCycle [

MorphicUIManager uiProcess == Processor activeProcess ifTrue: [
MorphicRenderLoop new doOneCycle ]
]

{ #category : #'host - testing' }
BlMorphicSteppingHost >> supportsFormSurface [

Expand Down
Loading