GitLab

https://crrev.com/c/915442 restructured the ownership of objects in app
banner Java code. In particular, InstallerDelegate was moved from
AppBannerInfoBarDelegateAndroid to AppBannerUiDelegate.

For native apps, InstallerDelegate is given
AppBannerInfoBarDelegateAndroid as an observer, and tells it about the
progress of installation. Prior to https://crrev.com/c/915442, when
AppBannerInfoBarDelegateAndroid was destroyed, it would also destroy the
InstallerDelegate, ensuring that it could not be signalled after
destruction. However, there is now the following potential for the
following sequence of (racy) actions:

1. C++ AppBannerInfoBarDelegateAndroid is destroyed. In its destructor,
   destroy() is called on its Java counterpart, resetting the native
   pointer on the Java side.
2. Race: prior to the AppBannerUiDelegate owned by the
   C++ AppBannerInfoBarDelegateAndroid being destroyed, the
   InstallerDelegate owned by AppBannerUiDelegate calls
   onInstallFinished on the Java-side AppBannerInfoBarDelegateAndroid.
   This will eventually call OnInstallFinished.
3. Meanwhile, the C++ AppBannerInfoBarDelegateAndroid's destructor
   continues, clearing the |ui_delegate_| field. This destroys the
   AppBannerUiDelegate and its owned InstallerDelegate.
4. The C++ AppBannerInfoBarDelegateAndroid has OnInstallFinished called
   on it (from 2), which attempts to access |ui_delegate_| and crashes
   with a segmentation fault.

This CL fixes the bug by moving 3 (clearing the |ui_delegate_| field)
to happen before 1 (calling destroy() on the Java-side
AppBannerInfoBarDelegateAndroid). This ensures the InstallerDelegate
is destroyed before the native pointer on the Java-side
AppBannerInfoBarDelegateAndroid is reset, so that any further progress
in installation is not signalled as destruction continues. This prevents
accessing |ui_delegate_| after it is cleared, and prevents the crash.

BUG=819434

Change-Id: Iab0f7cb9044da332b6d4c6ab0a6f1d41a72195ca
Reviewed-on: https://chromium-review.googlesource.com/959409
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542688}