JS::Heap::isSetToCrashOnTouch() unusable

RESOLVED FIXED in Firefox 52

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ptomato, Assigned: jonco)

Tracking

31 Branch
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36

Steps to reproduce:

Here's a code snippet that seems like it should work:

auto heap_wrapper = new JS::Heap<JSObject *>();
heap_wrapper->setToCrashOnTouch();
if (heap_wrapper->isSetToCrashOnTouch())
    printf("Indeed, set to crash on touch!\n");


Actual results:

Here's the compiler output from g++ 4.9.2:

In file included from /usr/local/include/mozjs-31/js/CallArgs.h:38:0,
                 from /usr/local/include/mozjs-31/jsapi.h:24,
                 from jssample.cpp:1:
/usr/local/include/mozjs-31/js/RootingAPI.h: In instantiation of ‘bool JS::Heap<T>::isSetToCrashOnTouch() [with T = JSObject*]’:
jssample.cpp:48:47:   required from here
/usr/local/include/mozjs-31/js/RootingAPI.h:257:20: error: no match for ‘operator==’ (operand types are ‘JSObject*’ and ‘JS::Heap<JSObject*>::<anonymous enum>’)
         return ptr == crashOnTouchPointer;



Expected results:

It looks like the template of isSetToCrashOnTouch() is missing a cast, it should probably have the same cast as setToCrashOnTouch() a few lines above it.

Note, I am testing this with ESR 31, but this code is still present in the latest ESR.
Created attachment 8807515 [details] [diff] [review]
bug1315122-remove-heap-crash-on-touch

This API is broken, but it's also unused.  Here's a patch to remove it.
Assignee: nobody → jcoppeard
Attachment #8807515 - Flags: review?(jdemooij)
Attachment #8807515 - Flags: review?(jdemooij) → review+

Comment 2

2 years ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/24ed6f84d823
Remove JS::Heap<T>'s unused setToCrashOnTouch() methods r=jandem

Comment 3

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/24ed6f84d823
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.