76758 - Need a docShell tree enumerator

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P3)

Description

[Simon Fraser [no longer active]]

We have a bunch of places in the code that need to iterate through the docShell 
hierarchy (e.g. find in frames, editor frameset stuff and more). I think it would 
be swell if we had an iterator over docShells. This needs to be a bidirectional 
iterator, and it should be possible to specify whether you want to include chrome 
and/or content docShells, and to confine the iteration to a particular subtree of 
the hierarchy.

Comment 1

[Adam Lock]

What do you intend to use it for? I can see it being reasonably be safe in a 
read-only capacity, but calling LoadURI or manipulating the DOM on them may lead 
to complications if docshells are being added or destroyed.

Comment 2

[kinmoz]

Having a docshell iterator/enumerator would simplify the code needed to do 
"find/replace" across the content area's frame/iframe hierarchy.

Comment 3

[Simon Fraser [no longer active]]

Mine.

Comment 4

[Simon Fraser [no longer active]]

Attachment: nsIDocShellEnumerator.idl

Comment 5

[Simon Fraser [no longer active]]

Attachment: nsDocShellEnumerator.h

Comment 6

[Simon Fraser [no longer active]]

Attachment: nsDocShellEnumerator.cpp

Comment 7

[Simon Fraser [no longer active]]

Attachment: nsDocShellModule changes

Comment 8

[Simon Fraser [no longer active]]

See attachments. I made it so that the interface is direction-agnostic, and you 
control whether you want a forwards or backwards iterator by using a different 
CID/contractID at CreateInstance time.

In either case, the enumerator will hand back items in pre-order, depth-first 
search order.

Reviews, please.

Comment 9

[rpotts (gone)]

Is there some reason why the nsISimpleEnumerator interface is not sufficient, 
once the enumerator has been created?

I can see why the "constructor" would need extra info - like direction and 
docshell type...  But once the enumerator has been returned, it seems like the 
nsISimpleEnumerator methods will do...

Couldn't there be a GetDocShellEnumerator(...) method on nsIDocShell which took 
a direction and "flags" arguments and returned an nsISimpleEnumerator which 
walked the hierarchy?

-- rick

Comment 10

[Adam Lock]

We have so many trees of stuff in Mozilla it may be beneficial to 
produce some generic interfaces so any trees implementing them can be 
enumerated.

Comment 11

[Judson Valeski]

adam - I think that's the idea behind nsISimpleEnumerator.

I agree w/ Rick here. Seems like this could be an attrib on the docshell, which
returns a nsISimpleEnumerator.

Comment 12

[Adam Lock]

What I mean to say is that we should write a simple, generic tree node interface 
(e.g. nsIGenericTreeNode) and implement it on our docshell and webbrowser 
object. Then the function that creates the enumerator can call the methods off 
of that to build it. This is better than writing something specific for 
docshells that uses nsIDocShellTreeItem/nsIDocShellTreeNode to walk the tree.

Comment 13

[Simon Fraser [no longer active]]

Feel free to run off and make a generic tree item iterator; I don't have time to 
do that. I am willing to implement this using GetDocShellEnumerator() and 
returning an nsISimpleEmumerator.

So I'll add:

  nsISimpleEnumerator getDocShellEnumerator(in integer aDirection, in long 
inItemType);

to nsIDocShell? The enumerator would just enumerate that docShell and its 
children.

Comment 14

[Simon Fraser [no longer active]]

Damn, i hate our enumerators.

So I'd like to use nsISimpleEnumerator, but my implementation of the Find stuff 
needs to be able to reset the enumerator using ->First(). Is it just me, or do 
our enumerator APIs really suck?

Comment 15

[Judson Valeski]

oh mama you're opening a can of worms :). for starters you can look into my 
thread a few weeks back on porkjockeys about enumerator iface ambiguities :-). 
Using nsISimpleEnumerator (public and frozen), if you want a reset enumerator 
you have to ask the accessor method to turn out a new one for you. 
GetEnumerator() or your equiv is required to hand back a new one each time it's 
called. That's they only way you can legally reset the cursor.

Comment 16

[Simon Fraser [no longer active]]

Attachment: nsDocShell changes for enumerator

Comment 17

[Simon Fraser [no longer active]]

Latest patch shows changes to existing files for the enumerator. Changes in the 
implementation of nsDocShellEnumerator are not shown, but minor adjustments to 
convert to nsISimpleEnumerator.

Note that I added a "const long typeAll=0x7FFFFFFF" to nsIDocShellTreeItem. I 
think that's the correct place for it. nsIDocShell gained:

    const long ENUMERATE_FORWARDS  = 0;
    const long ENUMERATE_BACKWARDS = 1;

    nsISimpleEnumerator getDocShellEnumerator(in long aItemType, in long 
aDirection);

Comment 18

[Adam Lock]

r=adamlock

I think whoever uses the docshell must ensure they do not remove docshells 
during enumeration or the enumerator's internal array may contain bogus 
pointers.

Comment 19

[Simon Fraser [no longer active]]

It's generally assumed that the contract between enumerator client and enumerator 
assumes that the client doesn't do anything to change what's being enumerated 
over, so what you say is correct. Thanks!

Comment 20

[Simon Fraser [no longer active]]

Checked in.