Closed Bug 726978 Opened 8 years ago Closed 8 years ago

Remove useless NS_New(Native)LocalFile calls in nsBrowserApp.cpp

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla13

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch The fix (obsolete) — Splinter Review
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.
Attachment #596976 - Flags: review?(mh+mozilla)
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.
Attachment #596976 - Flags: review?(mh+mozilla) → review-
Attached patch The fix v2 (obsolete) — Splinter Review
Ahem, it always helps to attach the right patch...
Attachment #596976 - Attachment is obsolete: true
Attachment #596991 - Flags: review?(mh+mozilla)
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.
Attachment #596991 - Flags: review?(mh+mozilla) → review+
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)
Attachment #596991 - Flags: feedback-
Attached patch The fix v3Splinter Review
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).
Attachment #596991 - Attachment is obsolete: true
Attachment #597002 - Flags: review?(mh+mozilla)
Attachment #597002 - Flags: review?(mh+mozilla) → review+
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/0a2efcfd53ff
Whiteboard: [inbound]
Target Milestone: --- → Firefox 13
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
https://hg.mozilla.org/mozilla-central/rev/0a2efcfd53ff
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Component: Build Config → General
Product: Firefox → Firefox Build System
Target Milestone: Firefox 13 → mozilla13
You need to log in before you can comment on or make changes to this bug.