• Elly Fong-Jones's avatar
    views: introduce WidgetDelegate::TransferOwnershipOfContentsView · 1206dbae
    Elly Fong-Jones authored
    This change is the first step in rectifying the confusion around
    WidgetDelegate's ownership of its contents view, which is documented
    at length in WidgetDelegate's class definition. Specifically, this
    change makes it clear when it is *intended* that ownership of the
    contents view is taken from the WidgetDelegate.
    
    This is step 1 of this plan:
    1) Introduce WidgetDelegate::TransferOwnershipOfContentsView() and
       migrate to it in places that are trying to take ownership
    2) Introduce WidgetDelegate::SetContentsView(), which will either:
       a. Store the provided view in a unique_ptr, if it is not owned
          by client, or
       b. Store the provided view in a raw pointer, if it is owned by
          client
    3) Replace all existing overrides of GetContentsView() with uses of
       SetContentsView()
    4) Require that !owned_by_client() in SetContentsView(), which will
       likely require intense surgery of client classes
    5) Have SetContentsView() take and TransferOwnershipOfContentsView()
       return a unique_ptr<View> rather than a View*
    
    This multi-step plan avoids ever storing a view with owned_by_client
    in a unique_ptr, which is an extremely simple route to an accidental
    double-free.
    
    Bug: 1075649
    Change-Id: I3ee55423ba5e4e7b48a50a9f4e04299f0721d410
    Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2376449
    Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
    Reviewed-by: default avatarPeter Boström <pbos@chromium.org>
    Reviewed-by: default avatarScott Violet <sky@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#804322}
    1206dbae
widget.cc 51.8 KB