66434 - nsDocShell.cpp needs cleanup

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[timeless]

This bug is filed in response to bug 30375 which brought up the inconsistent 
bracing style of 
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp

Comment 1

[Adam Lock]

I'll take this. nsDocShell will be run through indent before 0.9.1 to clean it 
up.

Comment 2

[Adam Lock]

Reassign

Comment 3

[Adam Lock]

Indent flags to be used for formatting. Some manual fixup will necessary 
afterwards due to indent's ignorance of C++ and certain macro constructs. Sample 
formatted nsDocShell.cpp follows. Assuming the before and after nsDocShell.cpp 
produce the same binary, the indented version will be checked in without review.

--indent-level4
--line-length80
--brace-indent0
-nut (** no tabs **)
--dont-format-comments
--continue-at-parentheses
--blank-lines-after-procedures
--braces-after-struct-decl-line
--braces-on-if-line
--honour-newlines
--dont-star-comments
--leave-optional-blank-lines
--dont-space-special-semicolon
--no-space-after-parentheses
--space-after-cast
--space-after-for
--space-after-if
--no-space-after-function-call-names
--space-after-while

Comment 4

[Adam Lock]

Attachment: Formatted nsDocShell

Comment 5

[Adam Lock]

Attachment received a modicum of manual tweaking. I didn't cleanup the 
nsRefreshTimer class at the bottom which is why formatting is so screwed up 
looking.

Comment 6

[timeless]

 nsCOMPtr < nsIDocShellTreeItem > root;
i think you'll need to fix all template instances to:
 nsCOMPtr <nsIDocShellTreeItem> root;

 
    NS_IMETHODIMP nsDocShell::GetInterface(const nsIID & aIID, void **aSink)
^kill leading whitespace
    NS_IF_ADDREF(((nsISupports *) * aSink));
                       looks odd ^

NS_IMETHODIMP
    nsDocShell::SetDocLoaderObserver(nsIDocumentLoaderObserver *
                                     aDocLoaderObserver)
my personal preference would be:
NS_IMETHODIMP nsDocShell::SetDocLoaderObserver(
    nsIDocumentLoaderObserver *aDocLoaderObserver)
and similarly for all long function definition lines.

similarly for function calls
        nsresult res =
            nsComponentManager::CreateInstance(kDocumentCharsetInfoCID,
                                               NULL,
                                               NS_GET_IID
                                               (nsIDocumentCharsetInfo),
                                               getter_AddRefs
                                               (mDocumentCharsetInfo));
is harder to read than:
        nsresult res =
            nsComponentManager::CreateInstance(
                kDocumentCharsetInfoCID,
                NULL,
                NS_GET_IID(nsIDocumentCharsetInfo),
                getter_AddRefs(mDocumentCharsetInfo)
            );

        nsIDocShellTreeItem *child = (nsIDocShellTreeItem *) 
mChildren.ElementAt(i);    // doesn't addref the result
line too long, put the comment on the next line?

---code questions, probably for another bug, sorry---

    if (mScriptGlobal) {
        mScriptGlobal->SetDocShell(nsnull);
        mScriptGlobal = nsnull;
    }
    if (mScriptContext) {
        mScriptContext->SetOwner(nsnull);
        mScriptContext = nsnull;
    }
//ok, so mScriptContext should be nsnull, and unless it's SetOwner attacks 
mScriptGlobal, that should be nsnull, so why:
    mScriptGlobal = nsnull;
    mScriptContext = nsnull;

Comment 7

[Adam Lock]

Indent has some "problems" with C++. I'll have to go in afterwards and sort out 
templates (nsCOMPtr) and some of the issues with method headers.

Timeless, in response to your docshell query. Yes that code is redundant, I've 
noticed it myself before now.

Comment 8

[Adam Lock]

nsDocShell.h, nsDocShell.cpp & nsWebShell.h have all been cleaned up. Function 
args remain formatted the way they were in the sample attachment (i.e. lined up 
with the open parentheses), but I've gone through fixing the nsCOMPtr problem 
and the occasional indentation problem. nsWebShell.cpp looks reasonably sane so 
I'm not touching that.

Marking fixed.

Comment 9

[sairuh (rarely reading bugmail)]

feel free to vrfy this, or give to someone who could do so. thx!

Comment 10

[Adam Lock]

... Time passes ...

Verified!