Log an error, when insertVisualStateCallback is called on a destroyed WebView

Check for destroyed WebView before adding a callback in didNavigateMainFrame

Check to make sure at the time that didNavigateMainFrame occurs that
the WebView is not destroyed (in which case skip registering the
visual state listener callback for onPageCommitVisible).

BUG=481534
Committed: https://crrev.com/46473984c6887c2a7b5a8602ef5526e6d16b06ec
Cr-Commit-Position: refs/heads/master@{#327062}

[Tobias Sargeant, 2015-04-27 15:34:40]

tobiasjs@chromium.org changed reviewers:
+ boliu@chromium.org, torne@chromium.org

[boliu, 2015-04-27 15:45:57]

maybe a test?

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1029:
protected boolean isDestroyed() {
is this needed?

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2305: if
(isDestroyed()) {
Add a different method, instead of changing behavior for the public API.

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
File
android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
(right):

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java:70:
if (awContents != null && !awContents.isDestroyed()) {
This won't be needed either?

[Tobias Sargeant, 2015-04-27 16:03:09]

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2305: if
(isDestroyed()) {
On 2015/04/27 15:45:57, boliu wrote:
> Add a different method, instead of changing behavior for the public API.

We still have to entertain the possibility that didNavigateMainFrame() happens
before the WebView is destroyed, but the callback on the UI thread to insert the
visual state callback occurs after destroy(), in which case we could still cause
an exception without the app doing anything wrong.

[boliu, 2015-04-27 16:06:59]

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/1109833002/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwContents.java:2305: if
(isDestroyed()) {
On 2015/04/27 16:03:09, Tobias Sargeant wrote:
> On 2015/04/27 15:45:57, boliu wrote:
> > Add a different method, instead of changing behavior for the public API.
> 
> We still have to entertain the possibility that didNavigateMainFrame() happens
> before the WebView is destroyed, but the callback on the UI thread to insert
the
> visual state callback occurs after destroy(), in which case we could still
cause
> an exception without the app doing anything wrong.

That's fine.

I mean add a new method like insertVisualStateCallbackWithDestroyCheck to be
called by didNavigateMainFrame's posted task, instead of exposing "isDestroyed"
to outside this class or changing behavior for the public API.

[Tobias Sargeant, 2015-04-27 16:20:44]

On 2015/04/27 16:06:59, boliu wrote:

> I mean add a new method like insertVisualStateCallbackWithDestroyCheck to be
> called by didNavigateMainFrame's posted task, instead of exposing
"isDestroyed"
> to outside this class or changing behavior for the public API.

Ok. Done.

[boliu, 2015-04-27 16:29:35]

lgtm

[Tobias Sargeant, 2015-04-27 16:37:30]

The CQ bit was checked by tobiasjs@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-27 16:37:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1109833002/60001

[commit-bot: I haz the power, 2015-04-27 17:00:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-27 17:00:53]

Dry run: This issue passed the CQ dry run.

[Tobias Sargeant, 2015-04-27 17:03:39]

The CQ bit was checked by tobiasjs@chromium.org

[commit-bot: I haz the power, 2015-04-27 17:04:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1109833002/60001

[commit-bot: I haz the power, 2015-04-27 17:05:06]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-27 17:06:23]

Patchset 4 (id:??) landed as
https://crrev.com/46473984c6887c2a7b5a8602ef5526e6d16b06ec
Cr-Commit-Position: refs/heads/master@{#327062}