Bugzilla – Full Text Bug Listing
| Summary: | [debug view] An empty Debug view has an enabled "Find..." context menu action | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Helmut J. Haigermoser <helmut.haigermoser> | ||||||
| Component: | Debug | Assignee: | Pawel Piech <pawel.1.piech> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | trivial | ||||||||
| Priority: | P3 | CC: | Michael_Rennie, pawel.1.piech | ||||||
| Version: | 3.7.1 | Flags: | Michael_Rennie:
review+
|
||||||
| Target Milestone: | 3.8 M5 | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Helmut J. Haigermoser
CQ:WIND00085209 *setting version to 3.7.1* Let me know in case you need more information! :) Helmut Created attachment 208568 [details]
The empty view with only Cancel to get out of it...
Disabling actions based on view content isn't trivial with the flex hierarchy views, but with some work it's doable. I ended up refreshing the actions state after every delta and after every content update to the root element. The operation is quick if the enablement state isn't actually changed, so it shouldn't be a performance drag. I'll hold off with applying the patch until after the holidays though. Created attachment 208790 [details]
Patch with fix.
(In reply to comment #3) > Disabling actions based on view content isn't trivial with the flex hierarchy > views, but with some work it's doable. I ended up refreshing the actions state > after every delta and after every content update to the root element. The > operation is quick if the enablement state isn't actually changed, so it > shouldn't be a performance drag. > Thanks Pawel! :) Performance should definitely not suffer because of this but it sounds like you've got that aspect covered. What about the case where the enablement state is actually changed, will that case impact the speed in any negative form? Helmut I pushed out the fix. Mike, please take a look when you have a chance. (In reply to comment #5) > (In reply to comment #3) > > Disabling actions based on view content isn't trivial with the flex hierarchy > > views, but with some work it's doable. I ended up refreshing the actions state > > after every delta and after every content update to the root element. The > > operation is quick if the enablement state isn't actually changed, so it > > shouldn't be a performance drag. > > > > Thanks Pawel! :) > Performance should definitely not suffer because of this but it sounds like > you've got that aspect covered. What about the case where the enablement state > is actually changed, will that case impact the speed in any negative form? > > Helmut The enablement state should only change when the view goes from empty to non-empty (and back), so other painting on the screen would take much more time than updating the action. With my implementation, the action state could be updated much more often than that, and this would be a concern if each update would take a lot of time. (In reply to comment #6) > I pushed out the fix. Mike, please take a look when you have a chance. > > (In reply to comment #5) > > (In reply to comment #3) > > > Disabling actions based on view content isn't trivial with the flex hierarchy > > > views, but with some work it's doable. I ended up refreshing the actions state > > > after every delta and after every content update to the root element. The > > > operation is quick if the enablement state isn't actually changed, so it > > > shouldn't be a performance drag. > > > > > > > Thanks Pawel! :) > > Performance should definitely not suffer because of this but it sounds like > > you've got that aspect covered. What about the case where the enablement state > > is actually changed, will that case impact the speed in any negative form? > > > > Helmut > > The enablement state should only change when the view goes from empty to > non-empty (and back), so other painting on the screen would take much more time > than updating the action. With my implementation, the action state could be > updated much more often than that, and this would be a concern if each update > would take a lot of time. Thanks for fixing this Pawel! :) Helmut (In reply to comment #6) > I pushed out the fix. Mike, please take a look when you have a chance. The actions now correctly disable, but I am curious what the performance impact is. Especially the change: http://git.eclipse.org/c/platform/eclipse.platform.debug.git/diff/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/VirtualFindAction.java?id=720f9a16ef8703e119f9f1f5bd84a8789b1e91d3 which will now call out to (potentially) populate the view to get the child count. Pawel, had you given any thought to crafting some performance tests for the #update() call impact? got this exception testing: org.eclipse.core.runtime.AssertionFailedException: assertion failed: Ignoring unexpected call to IWorkbenchSiteProgressService.decrementBusy(). This might be due to an earlier call to this method. at org.eclipse.core.runtime.Assert.isTrue(Assert.java:110) at org.eclipse.ui.internal.progress.WorkbenchSiteProgressService.decrementBusy(WorkbenchSiteProgressService.java:423) at org.eclipse.debug.internal.ui.views.launch.LaunchView.viewerUpdatesComplete(LaunchView.java:1475) at org.eclipse.debug.internal.ui.viewers.model.TreeModelContentProvider$4.run(TreeModelContentProvider.java:720) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.debug.internal.ui.viewers.model.TreeModelContentProvider.notifyUpdate(TreeModelContentProvider.java:713) at org.eclipse.debug.internal.ui.viewers.model.TreeModelContentProvider.access$4(TreeModelContentProvider.java:708) at org.eclipse.debug.internal.ui.viewers.model.TreeModelContentProvider$3.run(TreeModelContentProvider.java:682) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:135) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4140) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3757) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2701) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2665) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2499) at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:679) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:668) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:352) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) at java.lang.reflect.Method.invoke(Unknown Source) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:624) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:579) at org.eclipse.equinox.launcher.Main.run(Main.java:1433) at org.eclipse.equinox.launcher.Main.main(Main.java:1409) Steps: I was testing going from empty (in the view) to content and to empty again when I noticed this exception show up (In reply to comment #10) I opened bug 369465 on this issue. Since that bug is most likely related to the flex viewer re factoring in Juno stream, could we close this bug out separaterly? (In reply to comment #11) > (In reply to comment #10) > > I opened bug 369465 on this issue. Since that bug is most likely related to > the flex viewer re factoring in Juno stream, could we close this bug out > separaterly? Sounds good. |