Last Comment Bug 67843 - crash when opening chrome windows and dialogs - DOM global window impl problems
: crash when opening chrome windows and dialogs - DOM global window impl problems
Status: RESOLVED FIXED
:
Product: Core Graveyard
Classification: Graveyard
Component: Embedding: GTK Widget (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Christopher Blizzard (:blizzard)
: Stuart Parmenter
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2001-02-06 11:58 PST by Christopher Blizzard (:blizzard)
Modified: 2012-04-05 00:46 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch ( gross hack ) (3.72 KB, patch)
2001-02-06 13:05 PST, Christopher Blizzard (:blizzard)
no flags Details | Diff | Splinter Review
Proposed fix (also gross hack, but hopefully less so than the previous one :-) (1.16 KB, patch)
2001-02-06 15:59 PST, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Splinter Review

Description Christopher Blizzard (:blizzard) 2001-02-06 11:58:30 PST
Right now if you fire up TestGtkEmbed and go to a non-existant site it should
pop up a dialog saying that the site couldn't be found.  Right now it's just
hanging.

The problem is in GlobalWindowImpl::SizeOpenedDocShellItem() calling
GlobalWindowImpl::GetTreeOwner(nsIBaseWindow **aTreeOwner).  The assumption that
the tree owner also implements nsIBaseWindow is a bad one.  In my case I'm
implementing the tree owner myself but I'm not implementing nsIBaseWindow - I'm
implementing nsIWebBrowserSiteWindow since it's the site window, not the
internal window.  And, I can't implement that interface on my object and forward
the methods since the names conflict between nsIWebBrowserSiteWindow and
nsIBaseWindow.

OK, any advice?  This is something that I need to fix before 0.8 goes out the door.
Comment 1 Christopher Blizzard (:blizzard) 2001-02-06 13:05:41 PST
Created attachment 24559 [details] [diff] [review]
patch ( gross hack )
Comment 2 Christopher Blizzard (:blizzard) 2001-02-06 13:10:50 PST
This patch lets me implement the rest of nIPrompt since the method signatures
for nsIWebBrowserSiteWindow just happen to be the same.  It's a hack but it's
needed for 0.8 so it will work.
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2001-02-06 15:59:21 PST
Created attachment 24598 [details] [diff] [review]
Proposed fix (also gross hack, but hopefully less so than the previous one :-)
Comment 4 Johnny Stenback (:jst, jst@mozilla.com) 2001-02-06 16:00:11 PST
Dan, what do you think about the last patch in this bug?
Comment 5 Christopher Blizzard (:blizzard) 2001-02-06 20:18:28 PST
I want to try and get this in for 0.8.  Dan, if you can look at this.  If you
don't have time or you aren't sure I can check in my work around into 0.8.  It's
ugly but safe.
Comment 6 Dan M 2001-02-06 22:12:34 PST
  I'm new to this stuff and I only have like a 70% confidence in what I'm about 
to say. And my tree is horked right now so I can't doublecheck, *and* I'm more 
familiar with the Windows embedding test apps. But...
  I believe the winEmbed test app doesn't choke on this because its mDocShell is 
implemented by embedding/browser/webBrowser/nsDocShellTreeOwner, which is also an 
nsIBaseWindow. nsDocShellTreeOwner (that is, the implementation of 
nsIDocShellTreeOwner) is a Mozilla-supplied thing to be linked into an embedding 
app, while nsIWebBrowserChrome is a public API, its implementation supplied by 
the embedding app. In the embedding API review meetings I've been attending, 
they've been careful to draw that distinction. And I've been checking in code to 
help define it.
  I gather that GtkMozEmbedChrome is the top-level window; the thing provided by 
the embedding app. It should probably be different from the object that 
implements nsIDocShellTreeOwner; the thing provided browser/webBrowser.
  So like I said I'm not completely sure, but I think long-term you want to break 
GtkMozEmbedChrome up. Short term, I think your (Christopher's) patch is more 
correct than Johnny's.
Comment 7 Johnny Stenback (:jst, jst@mozilla.com) 2001-02-06 22:48:05 PST
r=jst for blizzard's patch.
Comment 8 Mike Shaver (:shaver -- probably not reading bugmail closely) 2001-02-06 22:57:16 PST
I love null-safety. sr=shaver
Comment 9 Mike Shaver (:shaver -- probably not reading bugmail closely) 2001-02-06 23:02:07 PST
Check me out, reviewing the wrong patch (which is, from the sr= perspective,
also quite reasonable).  Chris' patch also passes muster. sr=shaver.
Comment 10 Christopher Blizzard (:blizzard) 2001-02-06 23:05:55 PST
Checked in.  Thanks, guys.

Note You need to log in before you can comment on or make changes to this bug.