67876 - nsAReadableString::IsNull() and nsAWritableString::SetNull()

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[John Bandhauer]

For the DOM's use of strings we seem to want a concept of 'null' in addition to 
our concept of 'empty'. Since we use C++ references to strings, we want to be 
able to get and set the 'nullness' of a valid string object.

See the discussion in bug 66610

Is this something that might come to be?

Comment 1

[Scott Collins]

so, an `empty' string is any string that answers true to |IsEmpty| or |Length()
== 0|.  A `null' string is a special concept.  While any string might be empty,
only certain kinds of strings will support this alien notion of null-ness.
Strings are containers, not pointers.  Null-ness is a pointer concept, foreign
to containers.  This is a compromise solution.

My proposal is this, in the shared-buffer representation, a string pointing to a
zero-length buffer is empty; but a string pointing to no buffer is null.  You
can make a string be null by assigning a null string into it.  The natural
course of buffer sharing will cause the string on the lhs to get a null buffer.
 If the lhs is not the appropriate kind of string, it will act as though you are
assigning in an empty string.

The public test for null-ness will not be a member function (though, if
necessary, it could be supported by a private virtual function).

Comment 2

[Scott Collins]

As per discussion with jst, no explicit string interface will support the notion
of a NULL string (since strings are containers, not pointers), however, the
machinery is in place and can be accessed with functions like the following.
Johnny, feel free to put these in some convenient DOM header.

  inline
  PRBool
  IsNullDOMString( const nsAReadableString& aString )
    {
      return aString.GetBufferHandle()
               == NS_REINTERPRET_CAST(nsBufferHandle<char>*, 1);
    }

  inline
  PRBool
  IsNULLDOMString( const nsAReadableCString& aString )
    {
      return aString.GetBufferHandle()
               == NS_REINTERPRET_CAST(nsBufferHandle<PRUnichar>*, 1);
    }

This functionality will be supported in two ways.  First, a shared
representation string pointing to no buffer, will return |1| from it's
|GetBufferHandle| method.  Second, DOM specific subclasses, as suggested by
Johnny, can inherit from the sharing string, still share values, but return the
special value when they determine appropriate.

I'll attach a patch showing the comment change that went into
"nsPrivateSharableString.h" to document this facility.

Now the ball's in your court.

Comment 3

[Scott Collins]

Attachment: add comment describing the special exploitable behavior that allows `NULL' DOM strings

Comment 4

[Johnny Stenback (:jst)]

r=jst for the patch to add the comment.

Comment 5

[Alec Flett]

easy enough!
sr=alecf

Comment 6

[Scott Collins]

patch checked in

Comment 7

[Scott Collins]

OK, we're changing the rules a little bit; sorry.  Multiplexing the test for
null-ness with getting the real handle to the data has proved to be onerous. 
The new scheme is to provide some site-specific flags.  Mozilla can use one of
these flags to note null-ness, completely orthogonally to the inner workings of
strings and whether a string is sharable.

The following patch adds a method to ask for flags out of a string.  Specific
string clients (e.g., mozilla) can override to use flags as necessary.  The
default implementation lets you use a few of the bits in the flags word of a
shared buffer handle.  But you don't have to use that ... that's just a
convenient place to hang the default implementation.  If you need more bits, or
want flags for strings that aren't sharable, you now have the opportunity to do
the right thing.

The code in an early comment for |IsNullDOMString| should be changed accordingly.

This change is spurred by shaver's work in actually sharing JSStrings, my
corresponding work, and work in my branch that makes much heavier use of buffer
handles.

Again, apologies for any inconvenience.

Comment 8

[Scott Collins]

Attachment: adding machinery for ``site-specific'' string flags

Comment 9

[Scott Collins]

Included this fix in the branch, which has landed.  Fix checked in.