Fix a crash of the Content Shell for Android when showing videos in a ContentVideoView.

Fix a crash of the Content Shell for Android when showing videos in a ContentVideoView.

The Content Shell for Android crashes when it attempts to show a ContentVideoView(see below logs for details).
It only happens in the child Shell that newly opened, not in the initial Shell.
Because a ContentViewClient is set only for the initial Shell, but not the others.

Steps to reproduces:
1. Go to http://www.google.com and search YouTube.
2. Click the link of YouTube in the result pages. Then YouTube page is opened in a new Shell.
3. Click a play button on a video clip.

java.lang.NullPointerException
        at org.chromium.content.browser.ContentVideoView.showContentVideoView(ContentVideoView.java:176)
        at org.chromium.content.browser.ContentVideoViewLegacy.showContentVideoView(ContentVideoViewLegacy.java:128)
        at org.chromium.content.browser.ContentVideoView.<init>(ContentVideoView.java:150)
        at org.chromium.content.browser.ContentVideoViewLegacy.<init>(ContentVideoViewLegacy.java:57)
        at org.chromium.content.browser.ContentVideoView.createContentVideoView(ContentVideoView.java:378)
        at org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce(Native Method)
        at org.chromium.base.SystemMessageHandler.handleMessage(SystemMessageHandler.java:24)
        at android.os.Handler.dispatchMessage(Handler.java:102)
        at android.os.Looper.loop(Looper.java:136)
        at android.app.ActivityThread.main(ActivityThread.java:5017)
        at java.lang.reflect.Method.invokeNative(Native Method)
        at java.lang.reflect.Method.invoke(Method.java:515)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:779)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:595)
        at dalvik.system.NativeStart.main(Native Method)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252530

[Ted C, 2014-02-20 00:54:08]

Nice catch!  Some small refactoring suggestions, but looks good.

https://codereview.chromium.org/172043002/diff/1/content/shell/android/java/s...
File content/shell/android/java/src/org/chromium/content_shell/Shell.java
(right):

https://codereview.chromium.org/172043002/diff/1/content/shell/android/java/s...
content/shell/android/java/src/org/chromium/content_shell/Shell.java:89: public
void setContentViewClient(ContentViewClient client) {
with my comment in ShellManager, you should be able to delete this method and
just add ContentViewClient as a param to initialize.

Then you don't need to worry about clearing references or anything like that.

https://codereview.chromium.org/172043002/diff/1/content/shell/android/java/s...
content/shell/android/java/src/org/chromium/content_shell/Shell.java:271: if
(mContentViewClient != null)
mContentView.setContentViewClient(mContentViewClient);
we will know that mContentViewClient isn't null, so you can just set it without
this check.

https://codereview.chromium.org/172043002/diff/1/content/shell/android/java/s...
File content/shell/android/java/src/org/chromium/content_shell/ShellManager.java
(right):

https://codereview.chromium.org/172043002/diff/1/content/shell/android/java/s...
content/shell/android/java/src/org/chromium/content_shell/ShellManager.java:103:
public void setContentViewClient(ContentViewClient client) {
I would just have ShellManager build the ContentViewClient instead of the
Activity (getContext() can be cast to an Activity to construct the
ContentVideoViewClient).

Then you can get rid of this plumbing.

https://codereview.chromium.org/172043002/diff/1/content/shell/android/java/s...
content/shell/android/java/src/org/chromium/content_shell/ShellManager.java:115:
shellView.setContentViewClient(mContentViewClient);
just pass this param with initialize above.

[Sungmann Cho, 2014-02-20 02:43:41]

Thank you for your comments and suggestions.
I submit revised patches.

[Ted C, 2014-02-20 17:23:15]

On 2014/02/20 02:43:41, mann wrote:
> Thank you for your comments and suggestions.
> I submit revised patches.

lgtm!

[Ted C, 2014-02-20 17:23:25]

The CQ bit was checked by tedchoc@chromium.org

[commit-bot: I haz the power, 2014-02-20 17:28:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sungmann.cho@navercorp.com/172043002/7...

[commit-bot: I haz the power, 2014-02-20 19:10:46]

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

[commit-bot: I haz the power, 2014-02-20 19:10:46]

Retried try job too often on android_clang_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[Sungmann Cho, 2014-02-20 21:51:42]

The CQ bit was checked by sungmann.cho@navercorp.com

[commit-bot: I haz the power, 2014-02-20 21:53:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sungmann.cho@navercorp.com/172043002/7...

[commit-bot: I haz the power, 2014-02-20 22:55:37]

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

[commit-bot: I haz the power, 2014-02-20 22:55:37]

Retried try job too often on android_clang_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[Sungmann Cho, 2014-02-21 01:05:18]

The CQ bit was checked by sungmann.cho@navercorp.com

[commit-bot: I haz the power, 2014-02-21 01:10:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sungmann.cho@navercorp.com/172043002/7...

[commit-bot: I haz the power, 2014-02-21 02:33:00]

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

[commit-bot: I haz the power, 2014-02-21 02:33:01]

Retried try job too often on android_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[Sungmann Cho, 2014-02-21 03:38:31]

I omitted some import statements.
Sorry about that.

[Sungmann Cho, 2014-02-21 04:41:15]

The CQ bit was checked by sungmann.cho@navercorp.com

[commit-bot: I haz the power, 2014-02-21 04:41:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sungmann.cho@navercorp.com/172043002/5...

[Sungmann Cho, 2014-02-21 09:39:31]

The CQ bit was unchecked by sungmann.cho@navercorp.com

[Sungmann Cho, 2014-02-21 09:39:58]

The CQ bit was checked by sungmann.cho@navercorp.com

[commit-bot: I haz the power, 2014-02-21 09:40:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sungmann.cho@navercorp.com/172043002/5...

[commit-bot: I haz the power, 2014-02-21 10:02:21]

Change committed as 252530