Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 331932

Summary: 'Console' view disregards IHandlerService's deactivateHandler(IHandlerActivation) javadoc and passes in null
Product: [Eclipse Project] Platform Reporter: Remy Suen <remy.suen>
Component: DebugAssignee: Pawel Piech <pawel.1.piech>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: Michael_Rennie, pawel.1.piech, pwebster
Version: 3.7   
Target Milestone: 3.7 M5   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Proposed fix. none

Description Remy Suen CLA 2010-12-06 13:30:58 EST 
1. Put a breakpoint in org.eclipse.ui.internal.handlers.SlaveHandlerService's deactivateHandler(IHandlerActivation) for the parameter begin null.
2. Show the 'Console' view if you don't have it up already.
3. Prepare two hello world applications.
4. Put a breakpoint in one of them.
5. Debug the first one. The breakpoint is hit.
6. Now run the second one. Your outer's breakpoint will be hit.

NestableHandlerService(SlaveHandlerService).deactivateHandler(IHandlerActivation) line: 191	
	ProcessConsolePageParticipant.deactivated() line: 254	
	ConsoleView$8.run() line: 740	
	SafeRunner.run(ISafeRunnable) line: 42	
	ConsoleView.deactivateParticipants(IConsole) line: 738	
	ConsoleView.showPageRec(PageBookView$PageRec) line: 177	
	ConsoleView(PageBookView).partActivated(IWorkbenchPart) line: 756	
	ConsoleView$4.run() line: 404	
	RunnableLock.run() line: 35	
	UISynchronizer(Synchronizer).runAsyncMessages(boolean) line: 134	
	Display.runAsyncMessages(boolean) line: 4059	
	Display.readAndDispatch() line: 3678	
	Workbench.runEventLoop(Window$IExceptionHandler, Display) line: 2640	
	Workbench.runUI() line: 2604	
	Workbench.access$4(Workbench) line: 2438	
	Workbench$7.run() line: 671	
	Realm.runWithDefault(Realm, Runnable) line: 332	
	Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 664	
	PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 149
Comment 1 Remy Suen CLA 2010-12-06 13:47:45 EST 
Note that you must _not_ activate the 'Console' view while you are performing steps 4 to 6.
Comment 2 Pawel Piech CLA 2010-12-07 12:51:38 EST 
Created attachment 184738 [details]
Proposed fix.

I think there's just a test missing.  It solves the problem for me, I'm just not sure if it will have any bad side effects.
Comment 3 Pawel Piech CLA 2010-12-07 12:53:20 EST 
Michael, do you see any case where this test would cause deactivate not to be called properly?
Comment 4 Remy Suen CLA 2010-12-07 12:59:17 EST 
If you guys opt to put the null check into ProcessConsolePageParticipant _directly_ (like JavaStackTracePageParticipant), please also make sure you add a check for the 'fActivatedContext' field since that is also violating API rules by calling IContextService's deactivateContext(IContextActivation) method with a null parameter.
Comment 5 Pawel Piech CLA 2010-12-07 13:55:29 EST 
(In reply to comment #4)
> If you guys opt to put the null check into ProcessConsolePageParticipant ...
Thanks Remy.  A null check in ProcessConsolePageParticipant is a good idea as a precaution, though the root problem is that deacivate() is called even though activate() was not.
Comment 6 Michael Rennie CLA 2010-12-08 11:00:53 EST 
(In reply to comment #3)
> Michael, do you see any case where this test would cause deactivate not to be
> called properly?

It should be fine, I don't know of any code path that would have the part deactivated (fActive == false) and then try to show it.
Comment 7 Pawel Piech CLA 2011-01-24 14:34:06 EST 
Committed the fix.
Comment 8 Pawel Piech CLA 2011-01-24 14:34:53 EST 
(In reply to comment #6)
> It should be fine, I don't know of any code path that would have the part
> deactivated (fActive == false) and then try to show it.

Already reviewed.