Intended use of PropertyBag()?

30 views
Skip to first unread message

Jonathan Dixon

unread,
Aug 2, 2012, 10:17:47 PM8/2/12
to conten...@chromium.org
Hi content-team

I've been spending a bunch of time working through the way Chrome Android is currently mapping various delegate / callbacks that come out of the content layer over to Java side application objects in order to get the appropriate UI / policy decision.
Way back there was one big monster class hanging off TabContents's CoreTabHelperDelegate, so it could do something like TabContents::ForWebContents(w) and thus lookup a weak reference (jweak) to the java side object.

As we rework the code to be more beautiful and more layered, we have an increasing number of smaller java interfaces, defined in differing layers, that we need to be able to lookup references to instances off, keyed by WebContents*. The thinking had been to hold various maps around the place, but looking at it, storing the jweak' itself into a property on the WebContents::GetPropertyBag() would serve this well. The main requirement is the jweak is released cleanly when the WebContents is destroyed; looks like this will all be taken care of by using a simple scoped wrapper around that basic type. And this very tidily defines the object lifetimes without cramping the java GC's job, compared to the ad-hoc cross-language relationships that can get setup otherwise.

So my question is - is PropertyBag generally recommended for this sort of thing? To me it looks idea, but thought considering it in the bag I'd thought I'd just check there's no historical baggage around it or other recommended way to do this sort of thing.

Cheers
Jonathan


Avi Drissman

unread,
Aug 3, 2012, 12:11:14 AM8/3/12
to Jonathan Dixon, conten...@chromium.org
That's a great use of PropertyBag, and that's what it's there for. Right now it's really only used by TabContents, which uses the PropertyBag to hold a backref without using its ability to own. But it's definitely capable of more, so go for it.

Avi

Jonathan Dixon

unread,
Aug 3, 2012, 12:40:49 AM8/3/12
to Avi Drissman, conten...@chromium.org
Sweet.
Related question, is it intentional that some methods on WebContentsDelegate have no |source| WebContents* ? It throws spanners in my plans for those methods. e.g. 
  // Notifies the delegate that the page has made some progress loading....
  virtual void LoadProgressChanged(double progress) {}   // <== which page?
Some comments suggest the callback refers to the 'current active tab' but not all embedders necessarily have that concept.



Thanks

Avi Drissman

unread,
Aug 3, 2012, 1:21:15 AM8/3/12
to Jonathan Dixon, conten...@chromium.org
Um, yeah. This has... evolved. Feel free to fix those by adding WebContents*.

Avi
Reply all
Reply to author
Forward
0 new messages