The Views ownership model is complicated.  Usually, Views are owned by their parents, and AddChildView() is thus effectively an ownership transfer.  But this ownership transfer is implicit, both at the callsite and in the View itself (it holds children as raw pointers, not unique_ptrs).

This is confusing and error-prone.  Today, caller code typically looks like:

  View* blah = new View(blah);
  ... use |blah|
  parent_->AddChildView(blah);

It's not obvious here that AddChildView() transfers ownership and thus avoids a leak.  It's easy to introduce a leak with an early-return or other such change.  The code uses a bare new, which ideally I'd like to avoid long-term.

We could sort-of improve today's world by simply changing callers to something like:

  auto blah = base::MakeUnique<View>(blah);
  ... use |blah|
  parent_->AddChildView(blah.release());

...but this is more verbose than the current code, and the type system doesn't force you to do this, so it's still easy to avoid doing it.

Ideally, we should be in a world where we have something more like:

  // view.h:
  View* AddChildView(std::unique_ptr<View> child);

  // Caller:
  my_blah_ = parent_->AddChildView(base::MakeUnique<View>(blah));
  ... use |my_blah_|

This makes the ownership transfer clear.  More importantly, it makes the API difficult to misuse.

This isn't as simple as it sounds, because sometimes Views are owned elsewhere.  The set_owned_by_client() method toggles this state on.

So we first need to try and remove set_owned_by_client(), so Views are always owned by their parents.  This may not be easy, as doing this requires understanding precisely what the ownership model is for the existing callsites and whether/how it could be modeled differently.

Then we can change to use unique_ptrs for View's child pointers and the APIs that use them.

CCing some possibly-interested parties.