120026 - Rework PPEmbed so its classes are less dependent on each other

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Conrad Carlen (not reading bugmail)]

The design of PPEmbed is quite old and has some problems:
(1) The nsIWebBrowserChrome impl is owned by the CBrowserWindow class, meaning
there can be only 1 chrome for any number of web browsers in the window. We have
since required in the embedding APIs that there must be 1 chrome for each
browser. The patch makes that the case. CBrowserShell is a view class. It owns
and creates both the nsIWebBrowser and the nsIWebBrowserChrome objects, ensuring
a 1:1 relationship.
(2) Because the CBrowserWindow class created the chrome, it was what tied
together the chrome, the web browser, and the UI. Having to use these 3 classes
together creates too many dependencies. It should be possible to stick an
instance of CBrowserShell into any LWindow-derived window class. The patch makes
that possible. It breaks direct references between the the browser, the view,
and the window by using PowerPlant's LBroadcaster/LListener mechanism.

Comment 1

[Conrad Carlen (not reading bugmail)]

Attachment: patch

Adam - can you review from an embedding API perspective? Simon - can you sr?
There's a chunk of this patch you've already sr'd - the throbber code which
never made it in :-/

Comment 2

[Adam Lock]

Comment on attachment 64972 [details] [diff] [review]
patch

The patch seems fairly sane from an embedding perspective so r=adamlock.

I did notice that the impl of CBrowserChrome::SetDimensions  & GetDimensions is
wrong. It is possible for the caller to pass a combination of size & position
flags but the impl assumes it's either one or the other when it could be both
or none.

Comment 3

[Conrad Carlen (not reading bugmail)]

Attachment: partial patch based on Adam's comment

This impl of GetDimensions and SetDimensions allows getting/setting both
position and size at once. Simon, can you sr?

Comment 4

[Simon Fraser [no longer active]]

Comment on attachment 64972 [details] [diff] [review]
patch

sr=sfraser

Comment 5

[Conrad Carlen (not reading bugmail)]

Checked in.

Comment 6

[David Epstein]

verified in Mozilla trunk 0.9.8 Gecko/20020209, patch checkins:
PPBrowser.xml
ContextMenus.r
Throbber.r
ApplIDs.h
CBrowserApp.cp / .h
CBrowserChrome.cpp / .h 
CBrowserShell.cpp / .h
CBrowserWindow.cp / .h
CThrobber.cpp / .h
CWindowCreator.cpp / .h
PPEmbedConfig.h