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

Bug 356039

Summary: VE prints 'document.write("could not open %handler_name%");' when opening the RUI handlers
Product: z_Archived Reporter: fahua jin <jinfahua>
Component: EDTAssignee: Huo Zhen Zhong <huozz>
Status: CLOSED FIXED QA Contact:
Severity: critical    
Priority: P1 CC: chenzhh, huozz, jspadea, pharmon, svihovec
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
use new project environment for HTML gen in VE
none
Patch for default serialization store issue
none
new solution PreviewIREnvironment
none
fix for file locator
lasher: iplog+
HTML Gen: add context key to dependent js files
none
find js file by context key none

Description fahua jin CLA 2011-08-28 22:42:06 EDT
1) Create an RUI project.
2) Create an RUI handler.
3) Open the RUI handler with RUI editor, the expected result would be open the handler successfully, but now it prints document.write("could not open %handler_name%");

Note: please use the P2 installable build to test the build.
Comment 1 fahua jin CLA 2011-09-19 22:46:22 EDT
I raise the defect priority to Critical, it should be resolve ASAP!

Each time I have to clean the projects after restarting the workspace, and then re-open the handler to display the RUIHandler.
Comment 2 Huo Zhen Zhong CLA 2011-09-22 04:21:20 EDT
The problem is caused by the ve is opened before the compile get finished. A proper solution is to listen to the compile process, and refrash VE after compile finished
Comment 3 Tony Chen CLA 2011-09-22 09:48:17 EDT
How do we prevent this in RBD?
Comment 4 Brian Svihovec CLA 2011-09-22 22:53:24 EDT
I believe this error can be easily recreated, consistently, by turning off auto-build in the workspace and then creating a new RUIHandler.  When the VE Renders, it will always show a 'document.write' error.  

After looking into this issue, I believe the problem is that AbstractContentPRovider is not using the provided locator to find the contents of the part to render, and is relying on the current's project 'static' environment to find the part.  When Auto Build is disabled, the IR for the part is never created in the EGLbin directory, so the AbstractContentPRovider will never find a part using the 'static' environment.  When Auto Build is enabled, I believe we are seeing a race condition where the IR may exist if the builder has already run before the VE tries to render the part, or the IR may not yet exist, and a 'document.write' error is displayed. 

When working in the VE, the WorkingCopyGenerationOperation places temporary content for the part that is being edited in the .metadata directory of the workspace (e.g. C:/EDT/Development/EDT_HEAD/EDT_20110808/.metadata/.plugins/org.eclipse.edt.ide.rui/10, where 10 is the Context number of the current editor).  This directory is specified in the PreviewFileLocator, which is passed to AbstractContentProvider whenever a part is being rendered for a specific Context, and contains both the IR and the .js file for the part that is being edited.

I believe the solution to this problem will be to update AbstractContentPRovider::generateHTMLFile() so that it uses a temporary environment with an EGLPath that specifies the PreviewFileLocator directory followed by the EGLBin directory for the current project.  We also need to make sure that AbstractContentProvider uses the PreviewFileLocator when resolving .js files, or unsaved content will not render in the browser correctly.
Comment 5 Brian Svihovec CLA 2011-09-22 22:54:20 EDT
I have added Justin to this defect, as he may know how to easily create temporary environments that can be used in AbstractContentProvider::generateHTMLFile().
Comment 6 Justin Spadea CLA 2011-09-23 17:00:13 EDT
If you need to append additional IR locations to the project environment, then you'll need to create a new instance of ProjectEnvironment instead of going through ProjectEnvironmentManager. Maybe you could add a flag to ProjectEnvironmentManager.getProjectEnvironment() that forces it to create a new instance instead of using a cached version, so that you don't have to duplicate any initialization code - but don't cache this version!

Then I would add a method to ProjectEnvironment called "insertBuildPathEntry(int index)" that lets you insert a new IBuildPathEntry at index 0. You would have to implement this interface with a class that's capable of handling IRs in the .metadata directory used by the VE. Whatever you do, do not let your code alter the cached ProjectIREnvironments, as this will screw up the builder (e.g. do not register object stores with it - if that becomes necessary, you will need a new instance of ProjectIREnvironment as well).
Comment 7 Tony Chen CLA 2011-09-26 07:06:35 EDT
Created attachment 204001 [details]
use new project environment for HTML gen in VE
Comment 8 Tony Chen CLA 2011-09-26 07:11:47 EDT
I have released above patch to the code stream. It basically implements what Brian and Justin suggested. Use a new ProjectEnvironment for HTML Generation, this new ProjectEnvironment is composed on the fly and never cached. It has the context directory as its first BuildPathEntry. 

I have done a little bit of testing, it it working but still have problems sometimes. I could not not tell if the problems are caused by incompleteness of the fix or because of other RUI issues that are still open. 

Forest, you may want to take over and do more testing. 

Justin, if you have a chance, please review the code.
Comment 9 Tony Chen CLA 2011-09-26 07:15:47 EDT
I just noticed that the IRs are not created some times, (for example, when I create a new handler without autobuild on). When I looked at the context directory  (F:\workspace\eclipse\EDTDebug2\.metadata\.plugins\org.eclipse.edt.ide.rui\3\client), the ir file is an empty file. 

There could be another issue there.
Comment 10 Brian Svihovec CLA 2011-09-26 09:11:32 EDT
(In reply to comment #9)

Paul and Justin, under what conditions would an IR file be empty?  I was going to propose that there are other errors in the project that are keeping the IR from being created (WorkingCopyGenerationResult.hasError == true), but I think this would mean that the IR file wouldn't be created at all.
Comment 11 Justin Spadea CLA 2011-09-26 12:03:22 EDT
For the empty IR, I would add a "catch(Throwable t){t.printStackTrace();}" before the finally{} of WorkingCopyGenerationOperation line 177 - and stick a breakpoint on it. When it gets hit, do a Drop To Frame to re-step through and find what's going wrong.

Another thing to take a look at: ProjectEnvironment.setProjectBuildPathEntries(). This method doesn't check if the default serialization store for a particular key scheme has already been set (because previously it was guaranteed to only be 1 entry per project). Since you have two ProjectBuildPathEntries for the same 'root' project, with different output folders, this looks like the last one in will win. This isn't an issue right now since you're only reading IRs from this environment, but it could be a problem in the future if you start to use it for serialization.

Are there steps to reproduce the empty file? I played with it for a bit but the file always has contents.

As for the code change in CVS, it's different than what Brian and I expected. We did some talking and for now let's leave it as it is since it won't break the builder. There's another issue which might require some changes where we can revisit it: a RUI handler uses a record, they're in separate files, they've both been modified but not saved. There is a live IR for the handler but not for the record, meaning the preview will fail when trying to access the modified part of the record definition (e.g. field name changed).
Comment 12 Justin Spadea CLA 2011-09-26 12:04:29 EDT
Created attachment 204018 [details]
Patch for default serialization store issue
Comment 13 Tony Chen CLA 2011-09-27 11:11:22 EDT
The IR not created issue is found, adding a flush to the output stream will resolve it. 

After looking into more details, I realized the first patch I attached won't work. What we need seemed to be 
  A new Environment based on the WorkingCopyProjectEnvironment. If we use ProjectEnvironment, the part information is not there, so the Environment won't look for the IRs. (we may modify ProjectEnvironment to make it work, but seems use WorkingCopyProjectEnvironment is a better choice).
  The new environment will have the preview context directory as its first IR search directory. The proper way to do this seemed to be create a new implementation of IBuildPathEntry, and update ProjectEnvironment/WorkingCopyEnvironment to use it. 

Many of the VE unstable issues should be caused by missing this important design. I'll continue work on this tomorrow. Any suggestion would be very helpful.
Comment 14 Brian Svihovec CLA 2011-09-27 22:34:55 EDT
Tony, I do not have any updates, but I think we are on the right track.  Please post any updates as a patch to this defect and Justin and I will continue our reviews.  

Regarding the need to invoke 'flush', are we copying one stream to another?  I was under the impression that flush is implied when the stream is closed?
Comment 15 Tony Chen CLA 2011-09-28 09:36:30 EDT
Created attachment 204190 [details]
new solution PreviewIREnvironment
Comment 16 Tony Chen CLA 2011-09-28 09:47:41 EDT
Here's the new solution. If we look at how VE displays working copy content, here's the major steps
 1. When content change is detected, VE need to compile the working copy and save IRs to contxt directory. 
 2. The IRs needs to be generated into JavaScript using Dev mode JS generator
 3. When EVServer request the HTML, generate IR into HTML and send it back
 4. When EVServer request JS, find the JS in context directory and send back
 
In step 2 & 3, we need to invoke JSGenerator & HTMLGenerator which require an IREnvironment. While I have spend quite a while trying to create a proper ProjectEnvironment for VE, I found what really needed is just an IREnvironment. So in the new solution. I create a new IR environment PreviewIREnvironment (IEnvironment, File). This IR environment will populate itself with a ProjectIREnvironment plus the context directory as a FileSystemObjectStore. It is then used through step 1-3. The context directory object store is set as the setDefaultSerializeStore so that this IR environment can also be used to persist IR files (in step 1).  

PreviewIREnvironmentManager is used to manage creation of ProjectIREnvironment. 

Besides this, some more changes are made to WorkingCopyGenerationOperation. Originally, it generates JS for just the Handler part. Now it is generating JS for all parts in the file, which means if you have a record defined in the same class of handler, it can be used without need to save.  This still does not resolve the problem Justin mentioned (working copy content across 2 files), but I think we are one step close to that. 

The fix mention above is attached, and have been released to CVS. 

Forest is looking at some issues in step 4 in which VE does not look for js file in context directory. He will give a patch soon.
Comment 17 Huo Zhen Zhong CLA 2011-09-28 09:49:29 EDT
Created attachment 204191 [details]
fix for file locator
Comment 18 Huang Ji Yong CLA 2011-09-29 05:02:27 EDT
Created attachment 204268 [details]
HTML Gen: add context key to dependent js files
Comment 19 Huo Zhen Zhong CLA 2011-09-29 05:39:07 EDT
Created attachment 204277 [details]
find js file by context key
Comment 20 Huo Zhen Zhong CLA 2011-09-29 05:41:30 EDT
finally resloved
Comment 21 fahua jin CLA 2011-09-29 22:05:06 EDT
Verified in 0.7.0.v201109292101