Some favicon cleanup/upstreaming

Some favicon cleanup/upstreaming

- Add a check for whether or not the favicon is valid before caching it
- Still return an invalid favicon (will be default)
- Updated caching logic a bit to be more robust

BUG=

Committed: https://crrev.com/2414c180571df4258ade31bb2888a2c09b62ba81
Cr-Commit-Position: refs/heads/master@{#291989}

[David Trainor- moved to gerrit, 2014-08-22 21:57:48]

ptal!

[Yaron, 2014-08-22 22:11:12]

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

https://codereview.chromium.org/496733005/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:907: // If the
favicon is not yet valid (i.e. it's eitehr blank or a placeholder), then do
either

https://codereview.chromium.org/496733005/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:909: if (favicon !=
null && nativeIsFaviconValid(mNativeTabAndroid)) {
Instead of the extra JNI call, move this isfaviconvalid check into
nativeGetFavicon.

[David Trainor- moved to gerrit, 2014-08-25 17:49:31]

https://chromiumcodereview.appspot.com/496733005/diff/1/chrome/android/java/s...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://chromiumcodereview.appspot.com/496733005/diff/1/chrome/android/java/s...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:907: // If the
favicon is not yet valid (i.e. it's eitehr blank or a placeholder), then do
On 2014/08/22 22:11:12, Yaron wrote:
> either

Done.

https://chromiumcodereview.appspot.com/496733005/diff/1/chrome/android/java/s...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:909: if (favicon !=
null && nativeIsFaviconValid(mNativeTabAndroid)) {
On 2014/08/22 22:11:12, Yaron wrote:
> Instead of the extra JNI call, move this isfaviconvalid check into
> nativeGetFavicon.

Need to pull it out :(.  We still want to grab the favicon result in that case,
we just don't want to cache it (if we're matching downstream behavior).

[Yaron, 2014-08-25 19:19:29]

lgtm

https://chromiumcodereview.appspot.com/496733005/diff/20001/chrome/android/ja...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://chromiumcodereview.appspot.com/496733005/diff/20001/chrome/android/ja...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:909: if (favicon !=
null && nativeIsFaviconValid(mNativeTabAndroid)) {
Can you add a getter around nativeIsFaviconValid so we can remove this from
downstream too.

[David Trainor- moved to gerrit, 2014-08-25 22:15:37]

https://chromiumcodereview.appspot.com/496733005/diff/20001/chrome/android/ja...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://chromiumcodereview.appspot.com/496733005/diff/20001/chrome/android/ja...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:909: if (favicon !=
null && nativeIsFaviconValid(mNativeTabAndroid)) {
On 2014/08/25 19:19:29, Yaron wrote:
> Can you add a getter around nativeIsFaviconValid so we can remove this from
> downstream too.

I forgot to erase that method downstream.  It'll be gone when this lands.

[David Trainor- moved to gerrit, 2014-08-26 20:27:27]

The CQ bit was checked by dtrainor@chromium.org

[commit-bot: I haz the power, 2014-08-26 20:28:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dtrainor@chromium.org/496733005/20001

[commit-bot: I haz the power, 2014-08-26 21:52:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-26 21:56:17]

Committed patchset #2 (20001) as 62f929d3e6378b9c01698d18f1b7c041c308f516

[commit-bot: I haz the power, 2014-09-10 02:45:52]

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