Last Comment Bug 786386 - add way to pref off typed array move
: add way to pref off typed array move
Status: RESOLVED FIXED
[js:t][qa-]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla18
Assigned To: Steve Fink [:sfink] [:s:]
:
Mentors:
Depends on:
Blocks: 730873 803288
  Show dependency treegraph
 
Reported: 2012-08-28 11:28 PDT by Steve Fink [:sfink] [:s:]
Modified: 2012-10-18 14:46 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed
+
fixed


Attachments
Add easy way to disable typed array move() method (3.12 KB, patch)
2012-09-12 16:14 PDT, Steve Fink [:sfink] [:s:]
no flags Details | Diff | Splinter Review
Disable typed array move() command (949 bytes, patch)
2012-09-12 16:15 PDT, Steve Fink [:sfink] [:s:]
dmandelin: review+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review
Add easy way to disable typed array move() method (3.21 KB, patch)
2012-09-12 16:26 PDT, Steve Fink [:sfink] [:s:]
dmandelin: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Steve Fink [:sfink] [:s:] 2012-08-28 11:28:03 PDT
Bug 730873 adds a move() operation to typed arrays. It may need to be preffed off (see bug 730873 comment 20 and following). We should add a way to do that easily, and for now we'll plan on turning it off before release.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2012-08-29 13:57:33 PDT
Since this landed in the 16 cycle, it needs to be turned off before 16 ships, I believe.
Comment 2 Lukas Blakk [:lsblakk] use ?needinfo 2012-08-29 16:30:02 PDT
We'll track then to see that this is turned off, can someone be assigned to this and have a patch ready with approval nom?
Comment 3 Steve Fink [:sfink] [:s:] 2012-09-12 16:14:23 PDT
Created attachment 660605 [details] [diff] [review]
Add easy way to disable typed array move() method

Waldo would be the natural one for r?, but he's partially out right now.

This patch leaves move() enabled, but adds a way to disable in one line.
Comment 4 Steve Fink [:sfink] [:s:] 2012-09-12 16:15:09 PDT
Created attachment 660606 [details] [diff] [review]
Disable typed array move() command

And this one does the actual disable.
Comment 5 Steve Fink [:sfink] [:s:] 2012-09-12 16:26:24 PDT
Created attachment 660614 [details] [diff] [review]
Add easy way to disable typed array move() method
Comment 6 Steve Fink [:sfink] [:s:] 2012-09-13 09:58:32 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/e4ae0aa3d663
Comment 7 Steve Fink [:sfink] [:s:] 2012-09-13 09:58:56 PDT
Comment on attachment 660614 [details] [diff] [review]
Add easy way to disable typed array move() method

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 730873

User impact if declined: a move() method that is not part of the any spec will be exposed to web content, allowing web developers to start creating potential backwards-compatibility problems.

Testing completed (on m-c, etc.): local testing, very recently landed on m-i.
 
Risk to taking this patch (and alternatives if risky): extremely low. Applying this patch does not change the preprocessed code at all. The followup patch is needed to actually change something.

String or UUID changes made by this patch: none
Comment 8 Steve Fink [:sfink] [:s:] 2012-09-13 10:02:31 PDT
Comment on attachment 660606 [details] [diff] [review]
Disable typed array move() command

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 730873

User impact if declined: new method move() on typed arrays exposed to web content. move() is not yet part of any spec, so exposing it introduces backwards compatibility risk.

Testing completed (on m-c, etc.): local only (for now, this should only land on beta since we want to leave the new method enabled on aurora)

Risk to taking this patch (and alternatives if risky): Minimal
 
String or UUID changes made by this patch: none

This patch requires the other patch in this bug, which added the ENABLE_TYPEDARRAY_MOVE preprocessor macro.
Comment 9 Steve Fink [:sfink] [:s:] 2012-09-13 10:04:09 PDT
Requesting tracking flags for 17 and 18 as well, since this will still be an issue until move() is added to the spec or people agree that we can add it in.
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-09-13 18:59:01 PDT
https://hg.mozilla.org/mozilla-central/rev/e4ae0aa3d663
Comment 11 Steve Fink [:sfink] [:s:] 2012-09-16 09:39:57 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/e237933d2b08 (only the addition of the #define; still enabled on aurora)
Comment 13 Steve Fink [:sfink] [:s:] 2012-09-16 13:39:08 PDT
Messed up the rebase for beta.

https://hg.mozilla.org/releases/mozilla-beta/rev/2f8a5f915a15
Comment 14 Steve Fink [:sfink] [:s:] 2012-09-17 12:29:00 PDT
Scoobidiver: are those status markings accurate? This feature is *not* yet disabled for either 17 or 18, and we don't know yet know whether we want it to be enabled or disabled. To the naive observer (namely, me), tracking-firefox17+ and status-firefox17=fixed implies that nothing else needs to be done. Should I open a separate bug?
Comment 15 Scoobidiver (away) 2012-09-18 00:32:18 PDT
(In reply to Steve Fink [:sfink] from comment #14)
> Scoobidiver: are those status markings accurate?  This feature is *not* yet
> disabled for either 17 or 18, and we don't know yet know whether we want it
> to be enabled or disabled.
In Aurora, the changeset is http://hg.mozilla.org/releases/mozilla-aurora/rev/e237933d2b08 that matches the second patch.
In Beta, they are http://hg.mozilla.org/releases/mozilla-beta/rev/9faa4ad41e23 (second patch), http://hg.mozilla.org/releases/mozilla-beta/rev/b5c1c48202a7 (first patch), and http://hg.mozilla.org/releases/mozilla-beta/rev/2f8a5f915a15 (second patch partially backed out).

> tracking-firefox17+ and status-firefox17=fixed implies that nothing else
> needs to be done. Should I open a separate bug?
Yes because these patches are such a mess it's hard to follow.

Note You need to log in before you can comment on or make changes to this bug.