From bug 807757 comment 5: > +NOTE: Long ago there was be a "shutdown-cleanse" version of shutdown which was > +intended to clear profile data. This is no longer sent and observer code should > +remove support for it.

Does this also include the SHUTDOWN_CLEANSE and SHUTDOWN_PERSIST variables @ http://dxr.mozilla.org/mozilla-central/--GENERATED--/profile/public/_xpidlgen/nsIProfile.h.html#l60 ?

Updated patch and to tip

Comment on attachment 695409 [details] [diff] [review] Patch that removes references to shutdown-cleanse netwerk/cache/nsCacheService.cpp doesn't look right. You removed a call to nsCacheService::OnProfileShutdown completely, when I believe that you should only have removed the `cleanse` parameter to that method. In nsCertOverrideService.cpp and nsCookieService.cpp you have kept code that should have been removed (I think misread the "!NS_strcmp" call.)

(In reply to Benjamin Smedberg [:bsmedberg] from comment #4) > netwerk/cache/nsCacheService.cpp doesn't look right. You removed a call to > nsCacheService::OnProfileShutdown completely, when I believe that you should > only have removed the `cleanse` parameter to that method. I shouldn't have removed it. So, after reading profile-before-change on https://developer.mozilla.org/en-US/docs/Observer_Notifications again, I'm not sure what needs to be done for http://dxr.mozilla.org/mozilla-central/netwerk/cache/nsCacheService.cpp.html?string=nsCacheService.cpp#l2363 and #l2371 as 'cleanse' can't be true once shutdown_cleanse's removed > In nsCertOverrideService.cpp and nsCookieService.cpp you have kept code that > should have been removed (I think misread the "!NS_strcmp" call.) So, w.r.t nsCertOverrideService.cpp, I assume 'deletion of storage file' part at http://dxr.mozilla.org/mozilla-central/security/manager/ssl/src/nsCertOverrideService.cpp.html#l151 is not required anymore as it's handled elsewhere w.r.t nsCookieService.cpp, the if loop @ http://dxr.mozilla.org/mozilla-central/netwerk/cookie/nsCookieService.cpp.html#l1445 can be removed as 'shutdown-cleanse' is not part of the condition anymore Can you confirm?

remove the `cleanse` parameter and refactor the code as if it were never true. For certoverride, just remove that entire block (line 149-154)

Backed out for xpcshell failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/d802d6faa080 https://tbpl.mozilla.org/php/getParsedLog.php?id=19166337&tree=Mozilla-Inbound TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/extensions/cookie/test/unit/test_cookies_persistence.js | test failed (with xpcshell return code: 0), see following log: TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/extensions/cookie/test/unit/head_cookies.js | 4 == 1 - See following stack: TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/extensions/cookie/test/unit/head_cookies.js | caught exception 2147500036 - See following stack: TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/extensions/cookie/test/unit/test_cookies_thirdparty_session.js | test failed (with xpcshell return code: 0), see following log: TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/xpcshell/tests/extensions/cookie/test/unit/head_cookies.js | 4 == 1 - See following stack:

Try run for c93a65e68983 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=c93a65e68983 Results (out of 116 total builds): exception: 82 success: 26 warnings: 8 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/kshriram18@gmail.com-c93a65e68983

Latest Try Info: Results will be displayed on TBPL as they come in: https://tbpl.mozilla.org/?tree=Try&rev=99b8a9a3b478 Once completed, builds and logs will be available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/kshriram18@gmail.com-99b8a9a3b478

The removal of http://dxr.mozilla.org/mozilla-central/netwerk/cookie/nsCookieService.cpp#l1447 got rid of the RemoveAll() call which clears the cookies via hostTable.clear() in RemoveAllFromMemory Now this's reason the tests @ http://dxr.mozilla.org/mozilla-central/extensions/cookie/test/unit/test_cookies_persistence.js#l64 and thirdparty_session fail as seen in try log above. Now with support for 'shutdown-cleanse' removed, I assume that 'profile-before-change' doesn't guarantee removal of cookies. So, right thing to do would be to modify the cleanse part of the test files or call RemoveAll() based on a condition may be.

That part of the test is testing something which no longer exists, and we should just remove that part of the test.

Latest try's at https://tbpl.mozilla.org/?tree=Try&rev=105f547fe7cf

Mavericks, are you still working on this?

(In reply to Benjamin Smedberg [:bsmedberg] from comment #17) > Mavericks, are you still working on this? No. Apologies for the late reply. Someone else interested can take up this if it's still valid.

Can I please assigned for this bug?

Decheol, please feel free to create a patch for this bug. We typically don't assign bugs to new contributors until after they have submitted an initial patch.

Decheol Jeong, are you still planning on creating a patch for this? Or should I try and fix this bug instead?

I'am new to OPen source contribution. This is my first patch for this bug.

Comment on attachment 8547166 [details] [diff] [review] Bug-820613-fix There is trailing whitespace in this patch. Please remove this whitespace before checkin.

(In reply to abhi.workspace from comment #24) > Created attachment 8548268 [details] [diff] [review] > checkin needed This patch is missing your name, commit message, etc. To find out how to add them, see: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F While you're at it, can you also remove the paragraph about shutdown-cleanse in profile/notifications.txt[0]? [0]: http://mxr.mozilla.org/mozilla-central/source/profile/notifications.txt#59

checkin needed

Seeing as how this got backed out previously for test failures, a link to a green Try run would be much-appreciated :)

Looks like this was removed by Ehsan over in bug 1116545 at almost exactly the same time as this bug was being worked on, oddly enough.