Closed Bug 940194 Opened 11 years ago Closed 11 years ago

Build netwerk/cookie in unified mode

Categories

(Core :: Networking: Cookies, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: mz_mhs-ctb, Assigned: cpeterson)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files, 2 obsolete files)

No description provided.
Attached patch Patch (obsolete) — Splinter Review
Anyone else need to review this?
Attachment #8334345 - Flags: review?(ehsan)
I think this code change will be brittle because the unified network/cookie .cpp file will have two global variables called `gCookieService`: CookieChildService.cpp declares a CookieServiceChild* called gCookieService in the mozilla::net namespace and nsCookieService.cpp declares a nsCookieService* called gCookieService in the global namespace. nsCookieService.cpp also pulls the first gCookieService variable into the global namespace with `using namespace mozilla::net`.
I had a WIP patch for netwerk/cookie and, to avoid the gCookieService name conflicts, I renamed CookieServiceChild.cpp's gCookieService to sCookieServiceChild and made sCookieServiceChild a private static member of class CookieServiceChild. I also consolidated the duplicated behavior and pref constants into a shared definition in CookieServiceChild.h.
Comment on attachment 8334345 [details] [diff] [review] Patch Review of attachment 8334345 [details] [diff] [review]: ----------------------------------------------------------------- I agree with what Chris said. :-)
Attachment #8334345 - Flags: review?(ehsan) → review-
Attached patch Patch (v2) (obsolete) — Splinter Review
Attachment #8334345 - Attachment is obsolete: true
Attachment #8334967 - Flags: review?(ehsan)
Attachment #8334967 - Flags: review?(ehsan) → review+
Attachment #8334967 - Attachment is obsolete: true
Keywords: checkin-needed
Michael, are you planning to update your patch here? Thanks!
Flags: needinfo?(mz_mhs-ctb)
Yeah, might be a day or two though. :(
Flags: needinfo?(mz_mhs-ctb)
(In reply to comment #10) > Yeah, might be a day or two though. :( No rush, just wanted to check wit you to see if I need to pick this up! Thanks :-)
I guess you should take this. The linker is not being friendly to me right now...
Assignee: mz_mhs-ctb → ehsan
This patch was much simpler than I expected: nsCookieService.cpp can't be unified because it #defines FORCE_PR_LOG and, conveniently, all the symbol name conflicts were between nsCookieService.cpp and CookieChildService.cpp. No code changes are necessary.
Assignee: ehsan → cpeterson
Attachment #8341560 - Flags: review?(ehsan)
Comment on attachment 8341560 [details] [diff] [review] unify-netwerk-cookie.patch Review of attachment 8341560 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, that's what I suspected.
Attachment #8341560 - Flags: review?(ehsan) → review+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: