Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 342970 - Change signature of IPhase#execute() to #execute(Display)
Summary: Change signature of IPhase#execute() to #execute(Display)
Status: RESOLVED FIXED
Alias: None
Product: RAP
Classification: RT
Component: RWT (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 1.4 RC1   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 341763
  Show dependency tree
 
Reported: 2011-04-15 09:35 EDT by Rüdiger Herrmann CLA
Modified: 2011-05-03 06:06 EDT (History)
0 users

See Also:


Attachments
Proposed changes (19.43 KB, patch)
2011-04-20 10:23 EDT, Rüdiger Herrmann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rüdiger Herrmann CLA 2011-04-15 09:35:59 EDT
Currently all implementations of IPhase access the current display. I.e. their method body begins with
  Display display = RWTLifeCycle.getSessionDisplay();
  // do something with display

To avoid this duplication, I suggest to change the signature of the execute method to
  void execute( Display display );
and let the (single) caller pass in the display.

What do you think?
Comment 1 Rüdiger Herrmann CLA 2011-04-15 11:31:56 EDT
With bug 340927 resolved, the JavascriptResponseWriter (former HtmlResponseWriter) no longer throws IOExceptions. Hence there is no need any more for IPhase#execute() to declare throws IOException.
Comment 2 Rüdiger Herrmann CLA 2011-04-20 10:23:36 EDT
Created attachment 193703 [details]
Proposed changes

IPhase#execute() now requires the display on which it should operate as an argument.
PhaseExecutor (used by SimpleLifeCycle) now has an abstract getDisplay() method so that the LifeCycle implementation can supply the current session display (which can change from null to non-null and back with each phase).
Comment 3 Rüdiger Herrmann CLA 2011-05-03 06:06:12 EDT
Applied patch to CVS HEAD