Remove DocumentTabObserver and merge task description updates

Remove DocumentTabObserver and merge task description updates

This removes the DocumentTabObserver as a separate class inside
DocumentTab. It also add the favicon image as a param to
onFaviconUpdated inside TabObserver and uses that to update the
icon used for task descriptions to largest size in both Document
and WebAppActivities.

BUG=546182
Committed: https://crrev.com/8d22d29bfd0c021dd3d12eeff1369f34f6f8dd15
Cr-Commit-Position: refs/heads/master@{#361176}

[Yusuf, 2015-11-20 22:39:42]

yusufo@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Yusuf, 2015-11-20 22:39:46]

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

[commit-bot: I haz the power, 2015-11-20 22:40:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1464743005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1464743005/1

[gone, 2015-11-20 23:37:13]

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:2105: if
(!needUpdate && !FeatureUtilities.isDocumentMode(getApplicationContext()))
return;
This early-exit logic looks strange.  Shouldn't the TabObservers be what
determines whether or not something should be done?

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
(right):

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java:321:
if (mWebappInfo.icon() != null) return;
Why'd this new condition appear?  Seems like we should be updating the task
description, regardless of this.

[Yusuf, 2015-11-20 23:38:54]

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
(right):

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java:321:
if (mWebappInfo.icon() != null) return;
On 2015/11/20 23:37:13, dfalcantara wrote:
> Why'd this new condition appear?  Seems like we should be updating the task
> description, regardless of this.

See updateTaskDescription where if there is an icon inside WebappInfo we always
use that instead of the favicon. Having this early return here avoid making a
bitmap copy for sites that has an icon inside webappinfo.

[commit-bot: I haz the power, 2015-11-20 23:40:44]

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

[commit-bot: I haz the power, 2015-11-20 23:40:45]

Dry run: This issue passed the CQ dry run.

[Yusuf, 2015-11-20 23:47:34]

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java (right):

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java:2105: if
(!needUpdate && !FeatureUtilities.isDocumentMode(getApplicationContext()))
return;
On 2015/11/20 23:37:13, dfalcantara wrote:
> This early-exit logic looks strange.  Shouldn't the TabObservers be what
> determines whether or not something should be done?

Done.

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java
(right):

https://codereview.chromium.org/1464743005/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java:321:
if (mWebappInfo.icon() != null) return;
On 2015/11/20 23:38:54, Yusuf wrote:
> On 2015/11/20 23:37:13, dfalcantara wrote:
> > Why'd this new condition appear?  Seems like we should be updating the task
> > description, regardless of this.
> 
> See updateTaskDescription where if there is an icon inside WebappInfo we
always
> use that instead of the favicon. Having this early return here avoid making a
> bitmap copy for sites that has an icon inside webappinfo.

Done.

[gone, 2015-11-20 23:58:01]

lgtm

[Yusuf, 2015-11-23 19:30:38]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2015-11-23 19:35:20]

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

[commit-bot: I haz the power, 2015-11-23 20:40:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-23 20:41:53]

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