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

RESOLVED FIXED in Firefox 13

Status

()

Firefox
Build Config
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

unspecified
Firefox 13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
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-
(Assignee)

Comment 2

6 years ago
Created attachment 596991 [details] [diff] [review]
The fix v2

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-
(Assignee)

Comment 5

6 years ago
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).
Attachment #596991 - Attachment is obsolete: true
Attachment #597002 - Flags: review?(mh+mozilla)
Attachment #597002 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 6

6 years ago
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/0a2efcfd53ff
Whiteboard: [inbound]
Target Milestone: --- → Firefox 13

Comment 7

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
You need to log in before you can comment on or make changes to this bug.