Closed
Bug 487245
Opened 16 years ago
Closed 15 years ago
Cleanup and better organize code in widget/src/windows/nsWindow
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jimm, Assigned: jimm)
References
Details
Attachments
(1 file, 6 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
General goal: clean up and simplify code for debugging and improve commenting
- split off static functions into utility library
- split off platform specific code into separate files
- split off debug code into separate file
- split off closely related chunks of code into separate files
Assignee | ||
Comment 1•15 years ago
|
||
Splitting off large code chunks ended up not being needed after I got things well organized. I will probably split off paint into a separate file nsWindowGrx in prep for the opengl stuff we'll be doing. This seemed like a good starting point though before making any further changes.
Comment 2•15 years ago
|
||
The diff is somewhat hard to read. Do you have a list of what you're moving where and what you're simplifying?
Assignee | ||
Comment 3•15 years ago
|
||
(In reply to comment #2)
> The diff is somewhat hard to read. Do you have a list of what you're moving
> where and what you're simplifying?
It's broken down into major blocks and sub sections. The comment at the top has the outline that I followed -
Includes
Variables
Misc. utilities
nsIWidget impl.
nsIWidget methods
nsSwitchToUIThread impl.
nsSwitchToUIThread methods
Native message handling (main event loop)
Message processing methods
OnXXX event handlers
Paint handling
IME management and accessibility
Transparency
Popup hook handling
Child window impl.
In addition to the nsWindow reorg, I've split code out into other files -
nsWindowDefs.h
nsWindowDbg.h/.cpp
nsWindowCE.h/.cpp
I wasn't able to split out as much CE code as I had hoped, but maybe over time we can work on that. I avoided duplicate blocks of code in two files. That sort of restricted what I could move over to the ce src, but I was able to pull out about 500 unique lines.
Once I finish up I can post the raw src, that patch doesn't really do it justice IMHO.
Assignee | ||
Comment 4•15 years ago
|
||
Ok, this passed on try builds, including ce. I'll attach the nsWindow.cpp as well.
This patch looks a little scary, but 99.9% of it just code re-org and commenting cleanup.
Attachment #381154 -
Attachment is obsolete: true
Attachment #381325 -
Flags: review?(emaijala)
Assignee | ||
Comment 5•15 years ago
|
||
Assignee | ||
Comment 6•15 years ago
|
||
Added another block - "Event dispatch" for separating nsSwitchToUIThread and the event stuff better.
Attachment #381326 -
Attachment is obsolete: true
Assignee | ||
Comment 7•15 years ago
|
||
Here is the current layout of the code in nsWindow.cpp:
* Includes
* Variables
* Misc. utilities
* nsIWidget impl.
* nsIWidget methods
* nsSwitchToUIThread impl.
* nsSwitchToUIThread methods
* Event init and dispatch
* Native message handling
* Wnd proc
* Message processing
* OnXXX event handlers
* Paint handling
* IME management and accessibility
* Transparency
* Popup hook handling
* Child window impl.
Comment 8•15 years ago
|
||
Comment on attachment 381325 [details] [diff] [review]
windows nswindow reorg v.1
> \ No newline at end of file
Please fix these.
> * nsWindow has been reorganized into a set of major blocks
Make it "organized".
> * Blocks should be split out into seperate files if they
*separate*
I like it, but how about also putting the utility and WinCE functions into classes as statics so they wouldn't pollute the global namespace anymore? And while at it, why not put the global variables into nsWindow as statics? Currently we have a mess with both used arbitrarily.
Attachment #381325 -
Flags: review?(emaijala) → review-
Assignee | ||
Comment 9•15 years ago
|
||
(In reply to comment #8)
> (From update of attachment 381325 [details] [diff] [review])
> > \ No newline at end of file
>
> Please fix these.
>
> > * nsWindow has been reorganized into a set of major blocks
>
> Make it "organized".
>
> > * Blocks should be split out into seperate files if they
>
> *separate*
>
> I like it, but how about also putting the utility and WinCE functions into
> classes as statics so they wouldn't pollute the global namespace anymore? And
> while at it, why not put the global variables into nsWindow as statics?
> Currently we have a mess with both used arbitrarily.
Good ideas, will do.
Assignee | ||
Comment 10•15 years ago
|
||
ok, here's an update -
- more cleanup in nswindow.h/.cpp
- globals converted to statics
- standardized on sXXX and gXXX variable conventions
- split gfx related code out into nsWindowGfx
- wrapped ce and gfx utilities with classes
- misc. additional cleanup and commenting
Ere, this bit rots really often so if you could toss this on the top of your review queue I'd greatly appreciate it. :)
Attachment #381325 -
Attachment is obsolete: true
Attachment #381329 -
Attachment is obsolete: true
Attachment #383060 -
Flags: review?(emaijala)
Assignee | ||
Comment 11•15 years ago
|
||
Same patch with some added white space cleanup, primarily in the new headers.
Comment 12•15 years ago
|
||
Comment on attachment 383116 [details] [diff] [review]
windows nswindow reorg v.3
Yeahm, rots quickly and I'm too slow. Please unrot and get it in. We can continue work on it after that if necessary.
Attachment #383116 -
Flags: review+
Assignee | ||
Comment 13•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/1aecdc720018
I'm going to keep this bug open as I want to do some additional work on nsWindowGfx this week.
Assignee | ||
Comment 14•15 years ago
|
||
unbitrot and landed patch
Attachment #383060 -
Attachment is obsolete: true
Attachment #383116 -
Attachment is obsolete: true
Attachment #383060 -
Flags: review?(emaijala)
Assignee | ||
Updated•15 years ago
|
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Depends on: 483136
Depends on: 540510
You need to log in
before you can comment on or make changes to this bug.
Description
•