Refactor handleMessage() functionality in fullscreen

Refactor handleMessage() functionality in fullscreen
to static inner class to avoid HandlerLeak lint warnings

The following HandlerLeak lint warnings are reported that are legitimate:

(1) org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java
    This Handler class should be static or leaks might occur
    (org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager.2):
    HandlerLeak [warning]

(2) org/chromium/chrome/browser/fullscreen/FullscreenHtmlApiHandler.java
    This Handler class should be static or leaks might occur
    (org.chromium.chrome.browser.fullscreen.FullscreenHtmlApiHandler.1):
    HandlerLeak [warning]

The above warnings needs to fixed as the references to Handler class
might prevent outer class from getting garbage collected.

Making static inner class do not hold an implicit reference to the outer
class and hence this problem can be avoided. The same is also done for other
classes eg: TapGestureDetector.

BUG=None.

Committed: https://crrev.com/640ca0b5920679e4d0e8e8d53251e757d148e540
Cr-Commit-Position: refs/heads/master@{#311226}

[wajahat, 2015-01-07 07:55:11]

wajahat.s@samsung.com changed reviewers:
+ jaekyun@chromium.org, nyquist@chromium.org, tedchoc@chromium.org

[wajahat, 2015-01-07 07:55:11]

Ref: upstream fullscreen manager https://codereview.chromium.org/810853003

Pl. review. Thanks !

[Jaekyun Seok (inactive), 2015-01-07 08:43:52]

lgtm.

[wajahat, 2015-01-08 05:29:34]

On 2015/01/07 08:43:52, Jaekyun Seok wrote:
> lgtm.

Thanks Jaekyun

@Nyquist/Tedc could you pl. have a look, Thanks!

[Ted C, 2015-01-09 19:53:36]

lgtm

https://codereview.chromium.org/844473002/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java
(right):

https://codereview.chromium.org/844473002/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java:227:
public View getControlContainer() {
this doesn't need to be exposed (and shouldn't be public if it needed to be).

Just do:
chromeFullscreenManager.mControlContainer.requestLayout(); above.

[nyquist, 2015-01-10 01:27:05]

https://codereview.chromium.org/844473002/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java
(right):

https://codereview.chromium.org/844473002/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java:165:
ChromeFullscreenManager chromeFullscreenManager =
mChromeFullscreenManager.get();
I wonder, are there any cases whatsoever where this could return null?
For example if a message is posted to the handler, and then the last real
reference to the ChromeFullscreenManager was removed before the message was
handled here.

Or was the thinking behind this CL that ChromeFullscreenManager owns this
handler, so it will not be an issue as the handler would be GCed before the CFM?

Same goes for the other class, which uses a similar pattern.

[wajahat, 2015-01-12 08:06:46]

Review comments incorporated, PTAL! Thanks.

https://codereview.chromium.org/844473002/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java
(right):

https://codereview.chromium.org/844473002/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java:165:
ChromeFullscreenManager chromeFullscreenManager =
mChromeFullscreenManager.get();
On 2015/01/10 01:27:05, nyquist wrote:
> I wonder, are there any cases whatsoever where this could return null?
> For example if a message is posted to the handler, and then the last real
> reference to the ChromeFullscreenManager was removed before the message was
> handled here.
> 
> Or was the thinking behind this CL that ChromeFullscreenManager owns this
> handler, so it will not be an issue as the handler would be GCed before the
CFM?
> 
> Same goes for the other class, which uses a similar pattern.

The reasoning behind this CL was CFM would hold the handler, but on the safer
side i do agree we should validate for null and also for the other class
FSApiHandler.

https://codereview.chromium.org/844473002/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java:227:
public View getControlContainer() {
On 2015/01/09 19:53:36, Ted C wrote:
> this doesn't need to be exposed (and shouldn't be public if it needed to be).
> 
> Just do:
> chromeFullscreenManager.mControlContainer.requestLayout(); above.

Done.

[nyquist, 2015-01-12 22:42:30]

lgtm

[wajahat, 2015-01-13 05:21:47]

The CQ bit was checked by wajahat.s@samsung.com

[commit-bot: I haz the power, 2015-01-13 05:22:43]

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

[commit-bot: I haz the power, 2015-01-13 05:28:45]

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

[commit-bot: I haz the power, 2015-01-13 05:28:45]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[wajahat, 2015-01-13 06:04:56]

On 2015/01/13 05:28:45, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   ios_dbg_simulator on tryserver.chromium.mac
>
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

The below error looks to be unrelated to this patch, can anyone let me know
how to proceed for submitting this?

Traceback (most recent call last):
  File "/b/build/scripts/slave/runtest.py", line 2101, in <module>
    sys.exit(main())
  File "/b/build/scripts/slave/runtest.py", line 2062, in main
    result = _MainIOS(options, args, extra_env)
  File "/b/build/scripts/slave/runtest.py", line 1250, in _MainIOS
    result = _RunGTestCommand(command, extra_env, log_processor)
  File "/b/build/scripts/slave/runtest.py", line 204, in _RunGTestCommand
    command, pipes=pipes, parser_func=log_processor.ProcessLine, env=env)
  File "/b/build/scripts/common/chromium_utils.py", line 1021, in RunCommand
    stderr=subprocess.STDOUT, bufsize=0, **kwargs)
  File
"/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py",
line 711, in __init__
    errread, errwrite)
  File
"/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py",
line 1308, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

[Jaekyun Seok (inactive), 2015-01-13 06:09:45]

If so, you need to try again by clicking the commit button.

[wajahat, 2015-01-13 06:14:10]

The CQ bit was checked by wajahat.s@samsung.com

[commit-bot: I haz the power, 2015-01-13 06:14:59]

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

[commit-bot: I haz the power, 2015-01-13 06:23:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-13 06:24:33]

Patchset 2 (id:??) landed as
https://crrev.com/640ca0b5920679e4d0e8e8d53251e757d148e540
Cr-Commit-Position: refs/heads/master@{#311226}