Skip to content

Commit

Permalink
Media notification: If the current tab does not have favicon, don't t…
Browse files Browse the repository at this point in the history
…ry to fetch

If a page does not have any favicon, do not try to fetch an icon from
large icon bridge and wait for callback to update the notification icon.
We know if the page has favicon or not by tracking if onFaviconUpdated
was ever called.

BUG=827602

Change-Id: I3356470ca7088871540360621ee3140a3304621b
Reviewed-on: https://chromium-review.googlesource.com/1000314
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552506}
  • Loading branch information
ssiddhartha authored and Commit Bot committed Apr 20, 2018
1 parent 59445b2 commit 42ffcfb
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public class MediaSessionTabHelper implements MediaImageCallback {
private Bitmap mPageMediaImage;
@VisibleForTesting
Bitmap mFavicon;
// Set to true if favicon update callback was called at least once for the current tab.
private boolean mMaybeHasFavicon;
private Bitmap mCurrentMediaImage;
private String mOrigin;
@VisibleForTesting
Expand Down Expand Up @@ -444,6 +446,8 @@ private Activity getActivityFromTab(Tab tab) {
private void updateFavicon(Bitmap icon) {
if (icon == null) return;

mMaybeHasFavicon = true;

// Store the favicon only if notification is being shown. Otherwise the favicon is
// obtained from large icon bridge when needed.
if (isNotificationHiddingOrHidden() || mPageMediaImage != null) return;
Expand Down Expand Up @@ -537,6 +541,10 @@ private Bitmap getCachedNotificationImage() {
* @return if the favicon will be updated.
*/
private boolean fetchFaviconImage() {
// The page does not have a favicon yet to fetch since onFaviconUpdated was never called.
// Don't waste time trying to find it.
if (!mMaybeHasFavicon) return false;

if (mTab == null) return false;
WebContents webContents = mTab.getWebContents();
if (webContents == null) return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package org.chromium.chrome.browser.media.ui;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doCallRealMethod;

Expand Down Expand Up @@ -49,10 +51,16 @@ public class MediaNotificationFaviconTest extends MediaNotificationManagerTestBa
// Mock LargeIconBridge that runs callback using the given favicon.
private class TestLargeIconBridge extends LargeIconBridge {
private LargeIconCallback mCallback;
private boolean mGetIconCalledAtLeastOnce;

public boolean getIconCalledAtLeastOnce() {
return mGetIconCalledAtLeastOnce;
}

@Override
public boolean getLargeIconForUrl(
final String pageUrl, int desiredSizePx, final LargeIconCallback callback) {
mGetIconCalledAtLeastOnce = true;
mCallback = callback;
return true;
}
Expand Down Expand Up @@ -115,22 +123,34 @@ public void testGetNullNotificationIcon() {
TestLargeIconBridge largeIconBridge = new TestLargeIconBridge();
mTabHolder.mMediaSessionTabHelper.mLargeIconBridge = largeIconBridge;

// Simulate and hide notification.
mTabHolder.simulateMediaSessionStateChanged(true, false);
assertEquals(null, getDisplayedIcon());
mTabHolder.simulateMediaSessionStateChanged(false, false);

// Since the onFaviconUpdated was never called with valid favicon, the helper does not try
// to fetch favicon.
assertFalse(largeIconBridge.getIconCalledAtLeastOnce());
mTabHolder.simulateFaviconUpdated(mFavicon);

mTabHolder.simulateMediaSessionStateChanged(true, false);
assertEquals(null, getDisplayedIcon());

assertTrue(largeIconBridge.getIconCalledAtLeastOnce());
largeIconBridge.runCallback(null);
assertEquals(null, getDisplayedIcon());
}

@Test
public void testGetNotificationIcon() {
mTabHolder.simulateFaviconUpdated(null);
mTabHolder.simulateFaviconUpdated(mFavicon);
TestLargeIconBridge largeIconBridge = new TestLargeIconBridge();
mTabHolder.mMediaSessionTabHelper.mLargeIconBridge = largeIconBridge;

mTabHolder.simulateMediaSessionStateChanged(true, false);
assertEquals(null, getDisplayedIcon());

assertTrue(largeIconBridge.getIconCalledAtLeastOnce());
largeIconBridge.runCallback(mFavicon);
assertEquals(mFavicon, getDisplayedIcon());
}
Expand Down

0 comments on commit 42ffcfb

Please sign in to comment.