New issue
Advanced search Search tips

Issue 772211 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 771781



Sign in to add a comment

DISALLOW_NEW_EXCEPT_PLACEMENT_NEW is not working as expected

Project Member Reported by haraken@chromium.org, Oct 6 2017

Issue description

When DISALLOW_NEW_EXCEPT_PLACEMENT_NEW is specified, the object should be allowed only on the stack, HeapVector<Object> or HashTable<Object>.

The following case should not be allowed.

class Object { DISALLOW_NEW_EXCEPT_PLACEMENT_NEW(); };

class X : public GarbageCollected<X> {
  Object obj_;
};

However, it looks like the verification is not working as expected.

For example, MediaTrackCapabilities is put in ImageCapture.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/imagecapture/ImageCapture.h?q=ImageCapture&sq=package:chromium&dr=CSs&l=5

 
Blocking: 771781

Comment 2 by keishi@chromium.org, Oct 17 2017

haraken@: Is this dangerous? Do we want to ban this to keep the meaning of the macro strictly "allow only inline allocation"?
Yes. For example, we want to prevent people from allocating IDL dictionaries in an on-heap object (because IDL dictionaries may have v8::Local handles). IDL dictionaries should be allowed only on a stack or Vector<> on a stack.

> Do we want to ban this to keep the meaning of the macro strictly "allow only inline allocation"?

If we don't band it, we won't need DISALLOW_NEW_EXCEPT_PLACEMENT_NEW in the first place...? It should be the same as DISALLOW_NEW.


Comment 4 by keishi@chromium.org, Oct 17 2017

OK I understand now that DISALLOW_NEW_EXCEPT_PLACEMENT_NEW classes should be limited to on stack. Do I need to disallow only-placement-newable fields in stack-allocated classes?
Right.

STACK_ALLOCATED => Allowed only on a stack.

DISALLOW_NEW_EXCEPT_PLACEMENT_NEW  => Allowed only on a stack and in Vectors/HashTables.

Status: Assigned (was: Untriaged)

Sign in to add a comment