//content public API and use-after-destroy fixes

[Daniel Cheng]

I'm working on cleaning up the last few use-after-destroy bugs caught by msan's use-after-dtor checks.

One interesting case is in DlpContentManagerAshTest.CaptureModeInitWarnedCancelled. Here's the trimmed report:

==1575396==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x55d71bddbfe8 in content::WebContentsImpl::GetVisibility() content/browser/web_contents/web_contents_impl.cc:2866:3
    #1 0x55d72b2c494b in policy::DlpContentManagerAsh::MaybeChangeOnScreenRestrictions() chrome/browser/ash/policy/dlp/dlp_content_manager_ash.cc:397:22
    #2 0x55d72b2c4d83 in policy::DlpContentManagerAsh::OnWindowDestroying(aura::Window*) chrome/browser/ash/policy/dlp/dlp_content_manager_ash.cc:116:3
    #3 0x55d7325f54d2 in aura::Window::~Window() ui/aura/window.cc:202:14
    #4 0x55d7325f68bd in aura::Window::~Window() ui/aura/window.cc:191:19
    #5 0x55d71be773e4 in operator() third_party/libc++/src/include/__memory/unique_ptr.h:68:5
    #6 0x55d71be773e4 in reset third_party/libc++/src/include/__memory/unique_ptr.h:280:7
    #7 0x55d71be773e4 in content::WebContentsViewAura::~WebContentsViewAura() content/browser/web_contents/web_contents_view_aura.cc:685:11
    #8 0x55d71be77785 in content::WebContentsViewAura::~WebContentsViewAura() content/browser/web_contents/web_contents_view_aura.cc:677:45
    #9 0x55d71bdcd91a in operator() third_party/libc++/src/include/__memory/unique_ptr.h:68:5
    #10 0x55d71bdcd91a in reset third_party/libc++/src/include/__memory/unique_ptr.h:280:7
    #11 0x55d71bdcd91a in ~unique_ptr third_party/libc++/src/include/__memory/unique_ptr.h:249:71
    #12 0x55d71bdcd91a in content::WebContentsImpl::~WebContentsImpl() content/browser/web_contents/web_contents_impl.cc:1392:1

  Member fields were destroyed
    #0 0x55d7023559fd in __sanitizer_dtor_callback_fields /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/msan/msan_interceptors.cpp:1048:5
    #1 0x55d71bdcd5ea in ~WebContentsImpl content/browser/web_contents/web_contents_impl.h:2189:14
    #2 0x55d71bdcd5ea in content::WebContentsImpl::~WebContentsImpl() content/browser/web_contents/web_contents_impl.cc:1392:1

What's happening is:

- `visibility_` comes after `view_` in `WebContentsImpl`

- So by the time we destroy WebContentsViewAura, it's no longer valid to read the `visibility_` field

There are two easy ways to solve this, but both are somewhat problematic (for different reasons).

1. Reorder `visibility_` before `view_`. IMO, it's kind of weird for field ordering in content implementation classes to be driven by code that lives outside //content and is consuming it via the public API.

2. Change DlpContentManagerAsh to also check `IsBeingDestroyed()` before calling `GetVisibility()`. IMO, checking "is being destroyed" is pretty much always a code smell.

I'm leaning towards 1, with the general justification that POD fields should probably be ordered before fields that "own" stuff. In fact, I previously applied such a fix for the `is_being_destroyed_` field: https://chromium-review.googlesource.com/c/chromium/src/+/5574222

General thoughts?

Daniel

[Daniel Cheng]

Actually... I think I need to look into this a bit more. WebContentsDestroyed should have already fired, so it's a mystery why it's still in DlpContentManagerAsh::confidential_web_contents_.

Daniel