292036 - nsIProxyObjectManager.idl needs constants SYNC, ASYNC, ALWAYS for proxyType

Status: RESOLVED INCOMPLETE

(Core :: XPCOM, defect)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3
Build Identifier: 

nsIProxyObjectManager.idl needs constants SYNC, ASYNC, ALWAYS for proxyType

Currently proxyType constants are defined in nsProxyEvent.h, so they are
invisible to javascript.

http://lxr.mozilla.org/seamonkey/source/xpcom/proxy/public/nsIProxyObjectManager.idl#42
http://www.mozilla.org/projects/xpcom/Proxies.html
http://lxr.mozilla.org/seamonkey/source/xpcom/proxy/public/nsProxyEvent.h#56

Suggest adding to nsIProxyObjectManager.idl:

  /** Synchronous: Block until result available, like function call. **/
  const short INVOKE_SYNC   0x0001; 
  /** Asynchronous: Return without waiting for result.
      (Warning: do not pass &pointers into stack.) **/
  const short INVOKE_ASYNC  0x0002; 

  /** Always create proxy even if for same thread. **/
  const short CREATE_ALWAYS 0x0004;

(Renamed from PROXY_XXX so that 
 1. they are not replaced by preprocessor if used in code that includes
nsProxyEvent.h
 2. Make clearer that INVOKE_ASYNC and INVOKE_SYNC are not related to
CREATE_ALWAYS (when first learning about them, it is too easy to assume that
PROXY_ALWAYS means the union of PROXY_SYNC and PROXY_ASYNC.)


Reproducible: Always

Steps to Reproduce:

Comment 1

[gekacheka]

Attachment: add consts INVOKE_SYNC, INVOKE_ASYC, CREATE_ALWAYS

(patch -l -p 2 -i file.patch)

Patch uses short consts because 
http://www.mozilla.org/scriptable/xpidl/idl-authors-guide/rules.html#enum-and-const

says they can only be short or long, and a short should promote automatically.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 182274 [details] [diff] [review]
add consts INVOKE_SYNC, INVOKE_ASYC, CREATE_ALWAYS

These constants should be |long| to match the PRInt32 of the parameters they're
using.	Also, please don't request review on something that you haven't
actually compiled.  The syntax here is wrong and will cause xpidl to fall over.

Comment 3

[gekacheka]

Attachment: add consts INVOKE_SYNC, INVOKE_ASYC, CREATE_ALWAYS (untested)

(patch -l -p 2 -i file.patch)

Changed short to long, added '='.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 182682 [details] [diff] [review]
add consts INVOKE_SYNC, INVOKE_ASYC, CREATE_ALWAYS (untested)

sr=bzbarsky.  Darin, would you review?	I think it would be nice to get this
for 1.8...

Comment 5

[Darin Fisher]

Comment on attachment 182682 [details] [diff] [review]
add consts INVOKE_SYNC, INVOKE_ASYC, CREATE_ALWAYS (untested)

Please use proper doxygen style comment blocks:

/**
 * Synchronous: Block until result available, like function call.
 */

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 182682 [details] [diff] [review]
add consts INVOKE_SYNC, INVOKE_ASYC, CREATE_ALWAYS (untested)

requesting approval for adding some API comments here.

Comment 7

[Boris Zbarsky [:bzbarsky]]

CREATE_ALWAYS doesn't build on Windows; I renamed it to FORCE_PROXY_CREATION.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 182682 [details] [diff] [review]
add consts INVOKE_SYNC, INVOKE_ASYC, CREATE_ALWAYS (untested)

Can someone file a bug to change JS callers of that interface in the mozilla
tree to use these constants (there are a few...)?

Also, why duplicate the constants, instead of defining the C++ ones like:

#define PROXY_ASYNC nsIProxyObjectManager::INVOKE_ASYNC

thirdly: I think a better way to phrase the descriptions would be:
* Synchronous: Block until the function returns
* Always create a proxy, even if the target thread is the current thread.

Comment 9

[gekacheka]

Bug 303627 filed for callers.

(In reply to comment #8)
> Can someone file a bug to change JS callers of that interface in the mozilla
> tree to use these constants (there are a few...)?

Comment 10

[Benjamin Smedberg]

code is gone