Closed Bug 726978 Opened 11 years ago Closed 11 years ago

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


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: standard8, Assigned: standard8)




(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/
@@ -214,5 @@
>  else
>  	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:
Whiteboard: [inbound]
Target Milestone: --- → Firefox 13
Try run for a2be684fa890 is complete.
Detailed breakdown of the results available here:
Results (out of 31 total builds):
    success: 30
    warnings: 1
Builds (or logs if builds failed) available at:
Closed: 11 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.