519276 - revert some jsdIDebuggerService.idl IIDs

Status: VERIFIED WONTFIX

(Other Applications Graveyard :: Venkman JS Debugger, defect)

Description

[Bryan McQuade]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/532.1 (KHTML, like Gecko) Chrome/4.0.213.1 Safari/532.1
Build Identifier: 

https://bugzilla.mozilla.org/show_bug.cgi?id=136292 changed iids for several interfaces in jsdIDebuggerService.idl. Some of the changes were made for interfaces that did not change at all. Brendan Eich suggested that I open this bug to request that the unchanged interfaces have their iids rolled back.

The interfaces that should have their iids reverted are (portions of the diffs copied from http://hg.mozilla.org/mozilla-central/rev/36f4da6e262a):
    1.55 -[scriptable, uuid(54382875-ed12-4f90-9a63-1f0498d0a3f2)]
    1.56 +[scriptable, uuid(e391ba85-9379-4762-b387-558e38db730f)]
    1.57  interface jsdIFilterEnumerator : nsISupports

    1.64 -[scriptable, uuid(4c2f706e-1dd2-11b2-9ebc-85a06e948830)]
    1.65 +[scriptable, uuid(5ba76b99-acb1-4ed8-a4e4-a716a7d9097e)]
    1.66  interface jsdIScriptEnumerator : nsISupports

    1.73 -[scriptable, uuid(912e342a-1dd2-11b2-b09f-cf3af38c15f0)]
    1.74 +[scriptable, uuid(d96af02e-3379-4db5-885d-fee28d178701)]
    1.75  interface jsdIContextEnumerator : nsISupports

    1.82 -[scriptable, uuid(ae89a7e2-1dd1-11b2-8c2f-af82086291a5)]
    1.83 +[scriptable, uuid(cf7ecc3f-361b-44af-84a7-4b0d6cdca204)]
    1.84  interface jsdIScriptHook : nsISupports

    1.91 -[scriptable, uuid(f102caf6-1dd1-11b2-bd43-c1dbacb95a98)]
    1.92 +[scriptable, uuid(191d2738-22e8-4756-b366-6c878c87d73b)]
    1.93  interface jsdICallHook : nsISupports

   1.118 -[scriptable, uuid(9a7b6ad0-1dd1-11b2-a789-fcfae96356a2)]
   1.119 +[scriptable, uuid(3a722496-9d78-4f0a-a797-293d9e8cb8d2)]
   1.120  interface jsdIExecutionHook : nsISupports

   1.127 -[scriptable, uuid(a2dd25a4-1dd1-11b2-bda6-ed525acd4c35)]
   1.128 +[scriptable, uuid(3e5c934d-6863-4d81-96f5-76a3b962fc2b)]
   1.129  interface jsdIContext : jsdIEphemeral

Reproducible: Always

Comment 1

[Bryan McQuade]

Please fix in 1.9.2 (and 1.9.3, etc).

Comment 2

[Brendan Eich [:brendan]]

Thanks for filing.

timeless, can you take assignment and fix?

/be

Comment 3

[Brendan Eich [:brendan]]

(In reply to comment #1)
> Please fix in 1.9.2 (and 1.9.3, etc).

So bug 136292 is not marked blocking1.9.2 yet. Why does this need to be fixed on any repo other than mozilla-central?

/be

Comment 4

[Phil Ringnalda (:philor)]

Last I heard, timeless was in Kiev, or Krakow, or some other place starting with the letter "don't expect me to be able to do something mechanical that anyone who wants it done can do, any time soon."

It's on 1.9.2 because it landed last January, before 1.9.2 (much less blocking1.9.2) was a gleam in anyone other than the target milestone field's eye.

Comment 5

[Brendan Eich [:brendan]]

Ok, thanks.

/be

Comment 6

[Bryan McQuade]

I gave this a bit more thought and I think I know why timeless changed these IIDs.

Though these interfaces did not change, the IIDs of parameters passed to methods declared on these interfaces did change, because the signature of some methods on those interfaces changed.

For instance, we have

interface MyInterface : public nsISupports {
  void doSomething(in ChangedInterface obj);
};

MyInterface didn't actually change. However, ChangedInterface did.

MyInterface is the interface that clients will implement, whereas ChangedInterface instances are instantiated by the host Firefox instance and passed to the MyInterface instance.

So, if timeless did not change the IID of MyInterface, I could register a version of MyInterface in 1.9.2, and the host environment would happily accept that instance since it will successfully QI to the expected IID of MyInterface. However, when the host Firefox passes an instance of ChangedInterface to my instance, things will break since I expect ChangedInterface to implement the old (1.9.1) interface, whereas Firefox has passed me an instance of the new (1.9.2) interface.

If I QI the ChangedInterface before attempting to invoke methods on it I would be safe (fail gracefully for IIDs my code does not know about), but in general, extensions are unlikely to do so since they've already received a ChangedInterface pointer and thus it does not appear necessary to QI to verify that the IID matches.

So I think timeless did the right thing here.

It's not clear to me how to prevent this from happening in the general case. If we changed MyInterface to take an nsISupports instance instead of a ChangedInterface instance, and forced the client to QI to the ChangedInterface interface, then we'd be safe not changing the IID of MyInterface. But it's undesirable for clients to have to QI from nsISupports if the type of the object being passed in is known at compile time, so this is not a good general option.

In short I think this bug can be closed. I am curious, though, whether there is a better way to solve this problem. Timeless had to be mindful of this issue in order to catch it by changing those IIDs manually. Another developer might not catch this issue and could end up causing things to break as a result. Is there an easier way to enforce compatibility when dealing with unfrozen interfaces? Is it just up to the client to be very aggressive about validating IIDs of all method parameters when those params implement unfrozen IDL?

Comment 7

[timeless]

I'm glad bryan figured out why i did it. and I'm sorry that i wasn't reachable while i was traveling through Eastern Europe. I'm about 1500 bugmails behind atm because of my 3 week trip (and nearly two weeks trying to recover from it...).

As for automation, we definitely don't have any, which is why I did it manually.

It's actually not hard to get right (i.e. automatically detect this problem) using xpt_dump (or in fact the reflection interfaces which were at one time available to js).

Sadly, I'm exhausted, still have 1500 bugmails to read, and I'm traveling to Santa Clara, CA, US for TPAC next week, so worrying about how to write automation for this isn't something i could really work on for a while (realistically, not before January). that said, I could probably talk someone through it (especially someone like bryan who was able to figure out what i did and why w/ very little hinting).