Closed
Bug 728171
Opened 12 years ago
Closed 12 years ago
Make use of Scoped.h
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: Yoric, Assigned: Yoric)
References
Details
Attachments
(1 file, 11 obsolete files)
18.87 KB,
patch
|
Details | Diff | Splinter Review |
Bug 718938 introduces mfbt/Scoped.h and a number of general-purpose auto_ptr-style utilities. Let's use them!
Assignee | ||
Comment 1•12 years ago
|
||
Attaching an experimental patch. Everything compiles, tests in progress.
Assignee | ||
Comment 2•12 years ago
|
||
Ok, this seems to work.
Assignee | ||
Updated•12 years ago
|
Attachment #599147 -
Attachment is obsolete: true
Comment 3•12 years ago
|
||
Try run for 872d1fb41e05 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=872d1fb41e05 Results (out of 217 total builds): exception: 3 success: 180 warnings: 13 failure: 21 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dteller@mozilla.com-872d1fb41e05
Assignee | ||
Updated•12 years ago
|
Attachment #599252 -
Flags: feedback?(jones.chris.g)
Assignee | ||
Comment 4•12 years ago
|
||
Same patch, with slight clean-up in PluginModuleParent.cpp.
Attachment #600979 -
Flags: review?(jones.chris.g)
Assignee | ||
Updated•12 years ago
|
Attachment #599252 -
Attachment is obsolete: true
Attachment #599252 -
Flags: feedback?(jones.chris.g)
Comment on attachment 600979 [details] [diff] [review] Experimental patch >diff --git a/xpcom/glue/FileUtils.h b/xpcom/glue/FileUtils.h >+struct ScopedClosePRFDTraits >+ static void release(PRFileDesc* fd) { >+ if (fd) { >+ PR_Close(fd); >+ } This check is unnecessary. r=me with that fix.
Attachment #600979 -
Flags: review?(jones.chris.g) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Attaching a new version. I cleaned up FileUtils.h a little, and I propagating a tiny change from Scoped.h to dom/plugins/ipc/PluginModuleParent.cpp. \n Chris, do you think that we should land this patch once Scoped.h is landed?
Assignee | ||
Updated•12 years ago
|
Attachment #600979 -
Attachment is obsolete: true
yes phone
Assignee | ||
Comment 8•12 years ago
|
||
Attaching a variant matching a proposed change to Scoped.h without non-|const| |get()| and |operator->|. Which one do you think is best?
Assignee | ||
Comment 9•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #601907 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #601918 -
Attachment is obsolete: true
Assignee | ||
Comment 10•12 years ago
|
||
Updating to reflect the changes in Scoped.h and in the rest of m-c.
Assignee | ||
Comment 11•12 years ago
|
||
Updated to take into account changes in parent bug and to m-c. Chris, is your r+ sufficient to land this? Otherwise, who should I contact?
Attachment #612221 -
Attachment is obsolete: true
Attachment #613570 -
Flags: review?(jones.chris.g)
Updated•12 years ago
|
Attachment #613570 -
Flags: review?(jones.chris.g) → review+
Assignee | ||
Comment 12•12 years ago
|
||
Assignee | ||
Comment 13•12 years ago
|
||
Somehow, when I call a socket a "socker", they take offense.
Attachment #613910 -
Attachment is obsolete: true
Assignee | ||
Comment 14•12 years ago
|
||
Attachment #613960 -
Attachment is obsolete: true
Comment 15•12 years ago
|
||
Try run for 6a632ac82fe1 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=6a632ac82fe1 Results (out of 221 total builds): exception: 1 success: 187 warnings: 32 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dteller@mozilla.com-6a632ac82fe1
Assignee | ||
Comment 16•12 years ago
|
||
Attachment #613961 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 18•12 years ago
|
||
And a bustage fix for poor un-bitrotting on my part. https://hg.mozilla.org/integration/mozilla-inbound/rev/4058f038e97f
Comment 19•12 years ago
|
||
(In reply to Ryan VanderMeulen from comment #18) > And a bustage fix for poor un-bitrotting on my part. > https://hg.mozilla.org/integration/mozilla-inbound/rev/4058f038e97f Backed out per Yoric for bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/084cc1528d4f
Target Milestone: mozilla14 → ---
Updated•12 years ago
|
Keywords: checkin-needed
Comment 20•12 years ago
|
||
Please post an unbitrotted patch (preferably with a passing Try run).
Keywords: checkin-needed
Assignee | ||
Comment 21•12 years ago
|
||
I have some difficulties understanding what happened. I have just pulled and my patch seems to apply cleanly. Michael Wu pointed another error, tough, so I take the opportunity to fix it. According to TryServer, that patch builds on B2G. Currently running it on all platforms.
Attachment #614045 -
Attachment is obsolete: true
Comment 22•12 years ago
|
||
Try run for 6951c5ef44eb is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=6951c5ef44eb Results (out of 15 total builds): exception: 2 success: 11 warnings: 1 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dteller@mozilla.com-6951c5ef44eb
Comment 23•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0e03eb171e08
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Comment 24•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4058f038e97f backed out in https://hg.mozilla.org/mozilla-central/rev/084cc1528d4f.
Assignee | ||
Comment 25•12 years ago
|
||
I have difficulties following: is it in or out?
Assignee | ||
Comment 26•12 years ago
|
||
So the patch is actually out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 27•12 years ago
|
||
Ok, today, I finally have a merge issue. I suspect that the previous merge issue was only on inbound, not on master. Launching one (last?) TryServer run.
Comment 28•12 years ago
|
||
Try run for fda720573a26 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=fda720573a26 Results (out of 2 total builds): success: 2 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dteller@mozilla.com-fda720573a26
Comment 29•12 years ago
|
||
(In reply to David Rajchenbach Teller [:Yoric] from comment #26) > So the patch is actually out. To confirm: I backed out 4058f038e97f and 0e03eb171e08, so yes, this was backed out.
Comment 30•12 years ago
|
||
Try run for 85d3f2f97ab2 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=85d3f2f97ab2 Results (out of 15 total builds): success: 15 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dteller@mozilla.com-85d3f2f97ab2
Comment 32•12 years ago
|
||
*fingers crossed* (and landed by itself for easier backouts :P) https://hg.mozilla.org/integration/mozilla-inbound/rev/db23b2dac69c Ril.cpp still had bitrot issues, but I made sure to be more careful about it this time.
Keywords: checkin-needed
Comment 33•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/db23b2dac69c
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•