As a preliminary to bug 378776 (in which I would like to eliminate GetShellAt(0)), I think we should add the following API on nsIDocument:
virtual nsIPresShell *GetPrimaryShell() const = 0;
and implement this on nsDocument as follows:
return mShellsAreHidden ? nsnull :
The change all callers of GetShellAt(0) to calling GetPrimaryShell().
I can post a patch for the former part; taras says he can do the mass-change using his tools.
Created attachment 262792 [details] [diff] [review]
Comment on attachment 262792 [details] [diff] [review]
Created attachment 262821 [details] [diff] [review]
Since the number of lines affected by the patch is rather small it will be quicker to do a simple rename from GetShellAt to GetPrimaryShell(as done in attachment) and then modify the resulting patch by hand.
The file above shows the changes that would happen on linux. For the final patch I would combine linux & mac code, adjust the code by hand and check it on windows.
Alternatively, I can implement a parameter matching replace for squash by the end of the week and save the manual patch editing step.
That seems to be missing a lot of the callers (e.g. nsXULElement, etc), no? See http://lxr.mozilla.org/seamonkey/search?string=GetShellAt(0)
Created attachment 262847 [details] [diff] [review]
Used nsIDocument this time
Hmm. This still doesn't seem to have picked up all the callsites. e.g. the one in nsHTMLButtonElement::Click...
Note to self: to convert Taras's diff into what I want to check in:
cat ~/foo.patch | sed 's/GetPrimaryShell(0)/GetPrimaryShell()/' |
perl -pe 's/GetPrimaryShell\(([^)]+)\)/GetShellAt(\1)/'
Created attachment 263274 [details] [diff] [review]
This one has more stuff
Thanks for the bugreport. This was due to my assumption that the class hierarchy can be inferred from looking at class declarations. I forgot that templates can extend the hierarchy as nsDerivedSafe does.
Hope this one works.
Hmm. Yeah, that seems to get most of them. The things I think it should have caught but didn't seem to:
FreezeSubDocument in nsPresShell.cpp
All the other things it missed are in extensions/, mac widget, beos widget, seamonkey-only code, platform-specific accessibility code, and GTK embedding, which you probably just don't build.
I'll probably run with this version, but you might want to check on those three callsites to see why they didn't get caught.
Created attachment 263285 [details] [diff] [review]
more stuff yet
Sorry about those. I had stale .i files from last week which caused some confusion for the frontend.
Created attachment 263286 [details] [diff] [review]
Uploaded the wrong file.
So when I try to actually apply that diff (after changing the +++/--- lines to be relative to the srctree root), it fails to apply... Not sure why, exactly.
Weird. It only fails to apply if I actually modify the diff's +++/--- lines. If I use "patch -p5", it works fine.
Oh, and this change in nsDocument.cpp:
@@ -1980,2 +1980,1 @@
-nsDocument::GetShellAt(PRUint32 aIndex) const
+class nsIPresShell *nsDocument::GetShellAt(unsigned int aIndex) const
is not at all what we want; I backed it out.
Created attachment 263340 [details] [diff] [review]
Diff based on my original patch and Taras' changes
r+sr=me on Taras' stuff, by the way.
Created attachment 263371 [details] [diff] [review]
Now with all files
Hmm. This apparently missed one callsite in nsEventStateManager.cpp (while getting all the others in that file). Any idea why?
I blame CPP. #if defined(XP_WIN) || defined(XP_OS2)
That's why we need to add some serious hackery to oink to deal with platform specific macros. I have a two alternatives in mind, all are painful so I haven't gotten far.
a) Have a mac/windows/linux cluster + add support for ms compiler's C++ dialect to elsa(will take some work). Then have squash process 3x as much stuff as it does normally.
b) This one is harder but nicer. Add support for partial parsing, and integrate CPP into oink. Then we could process files and ignore includes that are outside of mozilla. Could run squash with any CPP flags needed then. Problem is that elsa wasn't designed to work that way so it's likely to be a big job.
If you have any ideas on how to tackle this, I'm all ears. This is gonna bite us in every single refactoring & analysis we do.
Of course there is also
c) Eliminate CPP conditionals in the moz source code :)
> I blame CPP. #if defined(XP_WIN) || defined(XP_OS2)
Oh, I missed that. Makes sense, then.