241568 - <nsFrame.cpp> cleanup

Status: RESOLVED WONTFIX

(Core :: Layout, task)

Description

[Serge Gautherie (:sgautherie)]

Fix for
{{ <http://tinderbox.mozilla.org/SeaMonkey/warn1082809200.7591.html>
anthonyd (2 warnings)
1.	layout/html/base/src/nsFrame.cpp:3879 (See build log excerpt)
	`nsIFrame*thisBlock' might be used uninitialized in this function
2.	layout/html/base/src/nsFrame.cpp:3908 (See build log excerpt)
	`nsIFrame*thisBlock' might be used uninitialized in this function
}} (NB: lines should be lines - 2, due to checkin "in progress")
will be included.

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) <nsFrame.cpp>

Also removes:
{{ <http://lxr.mozilla.org/mozilla/search?string=RefreshContentFrames>
RefreshContentFrames
/layout/html/base/src/nsFrame.cpp, line 121 -- static void
RefreshContentFrames(nsIPresContext* aPresContext, nsIContent * aStartContent,
nsIContent * aEndContent);
}}

(There could be more cleanup to do: but enough for now.)

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 146923 [details] [diff] [review]
(Av1) <nsFrame.cpp>


I have no compiler: Could you compile/test/review this patch ? Thanks.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Note that nsFrame.cpp has nothing to do with HTML Frames, and I'm a little wary
of anyone who doesn't know that touching this file....

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I already reviewed a patch (by the same author) for the warnings in comment 0 in
another bug.

Code cleanup and warning fixes are really only beneficial to the maintainers of
the code in question.  So is cvs blame.  Things like this mess up cvs blame, and
I'm not sure the benefits are worth it.

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> I already reviewed a patch (by the same author) for the warnings in comment 0 in
> another bug.

Are you sure ?
May be that was an other warning from bug 217089:
<http://bugzilla.mozilla.org/attachment.cgi?id=146892&action=view> !?

> Code cleanup and warning fixes are really only beneficial to the maintainers of
> the code in question.  So is cvs blame.  Things like this mess up cvs blame, and
> I'm not sure the benefits are worth it.

Well, there are some improvements too: let's wait for rbs's review...

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Av1b) <nsFrame.cpp>

Av1, "improved".

Comment 7

[rbs]

Comment on attachment 147072 [details] [diff] [review]
(Av1b) <nsFrame.cpp>

I am not really keen on this one, as it came from the coding style of the
initial coder. It is only worth the time/effort if you are taking ownership of
the file and asserting you style. [Note that the compiler will optimize most
away -- except, e.g., that |if view| with multiple calls -- which isn't
significant here]).

Comment 8

[Sylvestre Ledru [:Sylvestre]]

The code probably changed a lot in 10 years, closing