Last Comment Bug 726978 - Remove useless NS_New(Native)LocalFile calls in nsBrowserApp.cpp
: Remove useless NS_New(Native)LocalFile calls in nsBrowserApp.cpp
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 13
Assigned To: Mark Banner (:standard8)
:
Mentors:
Depends on: 686466
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-14 05:03 PST by Mark Banner (:standard8)
Modified: 2012-02-15 08:58 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
The fix (3.68 KB, patch)
2012-02-14 05:03 PST, Mark Banner (:standard8)
mh+mozilla: review-
Details | Diff | Splinter Review
The fix v2 (1.52 KB, patch)
2012-02-14 05:49 PST, Mark Banner (:standard8)
mh+mozilla: review+
bugspam.Callek: feedback-
Details | Diff | Splinter Review
The fix v3 (5.66 KB, patch)
2012-02-14 06:42 PST, Mark Banner (:standard8)
mh+mozilla: review+
Details | Diff | Splinter Review

Description Mark Banner (:standard8) 2012-02-14 05:03:25 PST
Created attachment 596976 [details] [diff] [review]
The fix

Bug 686466 made application.ini a static file. However, it missed removing some of the calls to NS_New(Native)LocalFile that were only required for reading the application.ini from disk when -app wasn't passed.

This patch removes that, it also removes some includes that are not required.
Comment 1 Mike Hommey [:glandium] 2012-02-14 05:07:50 PST
Comment on attachment 596976 [details] [diff] [review]
The fix

Review of attachment 596976 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/app/Makefile.in
@@ -214,5 @@
>  else
>  	$(RM) $(DIST)/$(MOZ_MACBUNDLE_NAME)/Contents/MacOS/$(PROGRAM)
>  	rsync -aL $(PROGRAM) $(DIST)/$(MOZ_MACBUNDLE_NAME)/Contents/MacOS
>  endif
> -	-cp -L $(DIST)/bin/mangle $(DIST)/bin/shlibsign $(DIST)/$(MOZ_MACBUNDLE_NAME)/Contents/$(APPFILES)

This does more than advertized. The nsBrowserApp.cpp part is also missing.
Comment 2 Mark Banner (:standard8) 2012-02-14 05:49:53 PST
Created attachment 596991 [details] [diff] [review]
The fix v2

Ahem, it always helps to attach the right patch...
Comment 3 Mike Hommey [:glandium] 2012-02-14 05:54:12 PST
Comment on attachment 596991 [details] [diff] [review]
The fix v2

Review of attachment 596991 [details] [diff] [review]:
-----------------------------------------------------------------

You want to do that in mobile/xul/app/nsBrowserApp.cpp, and b2g/app/nsBrowserApp.cpp, too.
Comment 4 Justin Wood (:Callek) (Away until Aug 29) 2012-02-14 06:21:59 PST
Comment on attachment 596991 [details] [diff] [review]
The fix v2

Nit if we're doing this, we no longer use exePath in do_main, might as well stop calling it with that. (We still need to calc it for the XPCOM startup)
Comment 5 Mark Banner (:standard8) 2012-02-14 06:42:57 PST
Created attachment 597002 [details] [diff] [review]
The fix v3

Addresses the various comments. I've also pushed to try to confirm at least the mobile changes are right (I just applied the patch there).
Comment 6 Mark Banner (:standard8) 2012-02-14 10:05:18 PST
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/0a2efcfd53ff
Comment 7 Mozilla RelEng Bot 2012-02-14 10:16:05 PST
Try run for a2be684fa890 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=a2be684fa890
Results (out of 31 total builds):
    success: 30
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bugzilla@standard8.plus.com-a2be684fa890
Comment 8 Marco Bonardo [::mak] 2012-02-15 08:58:40 PST
https://hg.mozilla.org/mozilla-central/rev/0a2efcfd53ff

Note You need to log in before you can comment on or make changes to this bug.