Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(483)

Side by Side Diff: chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java

Issue 2758753003: 📰 Refresh the tiles when a removal is undone (Closed)
Patch Set: address comments Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroupDelegateImpl.java » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2017 The Chromium Authors. All rights reserved. 1 // Copyright 2017 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 package org.chromium.chrome.browser.suggestions; 5 package org.chromium.chrome.browser.suggestions;
6 6
7 import android.content.Context; 7 import android.content.Context;
8 import android.content.res.Resources; 8 import android.content.res.Resources;
9 import android.graphics.Bitmap; 9 import android.graphics.Bitmap;
10 import android.graphics.BitmapFactory; 10 import android.graphics.BitmapFactory;
11 import android.graphics.Color; 11 import android.graphics.Color;
12 import android.graphics.drawable.BitmapDrawable; 12 import android.graphics.drawable.BitmapDrawable;
13 import android.support.annotation.Nullable; 13 import android.support.annotation.Nullable;
14 import android.support.v4.graphics.drawable.RoundedBitmapDrawable; 14 import android.support.v4.graphics.drawable.RoundedBitmapDrawable;
15 import android.support.v4.graphics.drawable.RoundedBitmapDrawableFactory; 15 import android.support.v4.graphics.drawable.RoundedBitmapDrawableFactory;
16 import android.view.ContextMenu; 16 import android.view.ContextMenu;
17 import android.view.ContextMenu.ContextMenuInfo; 17 import android.view.ContextMenu.ContextMenuInfo;
18 import android.view.LayoutInflater; 18 import android.view.LayoutInflater;
19 import android.view.View; 19 import android.view.View;
20 import android.view.View.OnClickListener; 20 import android.view.View.OnClickListener;
21 import android.view.View.OnCreateContextMenuListener; 21 import android.view.View.OnCreateContextMenuListener;
22 import android.view.ViewGroup; 22 import android.view.ViewGroup;
23 23
24 import org.chromium.base.ApiCompatibilityUtils; 24 import org.chromium.base.ApiCompatibilityUtils;
25 import org.chromium.base.Callback;
25 import org.chromium.base.Log; 26 import org.chromium.base.Log;
26 import org.chromium.base.VisibleForTesting; 27 import org.chromium.base.VisibleForTesting;
27 import org.chromium.chrome.R; 28 import org.chromium.chrome.R;
28 import org.chromium.chrome.browser.ChromeFeatureList; 29 import org.chromium.chrome.browser.ChromeFeatureList;
29 import org.chromium.chrome.browser.favicon.LargeIconBridge.LargeIconCallback; 30 import org.chromium.chrome.browser.favicon.LargeIconBridge.LargeIconCallback;
30 import org.chromium.chrome.browser.ntp.ContextMenuManager; 31 import org.chromium.chrome.browser.ntp.ContextMenuManager;
31 import org.chromium.chrome.browser.ntp.ContextMenuManager.ContextMenuItemId; 32 import org.chromium.chrome.browser.ntp.ContextMenuManager.ContextMenuItemId;
32 import org.chromium.chrome.browser.ntp.MostVisitedTileType; 33 import org.chromium.chrome.browser.ntp.MostVisitedTileType;
33 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; 34 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
34 import org.chromium.chrome.browser.widget.RoundedIconGenerator; 35 import org.chromium.chrome.browser.widget.RoundedIconGenerator;
35 import org.chromium.ui.mojom.WindowOpenDisposition; 36 import org.chromium.ui.mojom.WindowOpenDisposition;
36 37
37 import java.util.Arrays; 38 import java.util.Arrays;
38 import java.util.HashMap; 39 import java.util.HashMap;
39 import java.util.HashSet; 40 import java.util.HashSet;
40 import java.util.Map; 41 import java.util.Map;
41 import java.util.Set; 42 import java.util.Set;
42 43
43 /** 44 /**
44 * The model and controller for a group of site suggestion tiles. 45 * The model and controller for a group of site suggestion tiles.
45 */ 46 */
46 public class TileGroup implements MostVisitedSites.Observer { 47 public class TileGroup implements MostVisitedSites.Observer {
47 /** 48 /**
48 * Performs work in other parts of the system that the {@link TileGroup} sho uld not know about. 49 * Performs work in other parts of the system that the {@link TileGroup} sho uld not know about.
49 */ 50 */
50 public interface Delegate { 51 public interface Delegate {
51 void removeMostVisitedItem(Tile tile); 52 /**
53 * @param tile The tile corresponding to the most visited item to remove .
54 * @param removalUndoneCallback The callback to invoke if the removal is reverted. The
55 * callback's argument is the URL being res tored.
56 */
57 void removeMostVisitedItem(Tile tile, Callback<String> removalUndoneCall back);
52 58
53 void openMostVisitedItem(int windowDisposition, Tile tile); 59 void openMostVisitedItem(int windowDisposition, Tile tile);
54 60
55 /** 61 /**
56 * Gets the list of most visited sites. 62 * Gets the list of most visited sites.
57 * @param observer The observer to be notified with the list of sites. 63 * @param observer The observer to be notified with the list of sites.
58 * @param maxResults The maximum number of sites to retrieve. 64 * @param maxResults The maximum number of sites to retrieve.
59 */ 65 */
60 void setMostVisitedSitesObserver(MostVisitedSites.Observer observer, int maxResults); 66 void setMostVisitedSitesObserver(MostVisitedSites.Observer observer, int maxResults);
61 67
(...skipping 84 matching lines...) Expand 10 before | Expand all | Expand 10 after
146 @Nullable 152 @Nullable
147 private Tile[] mPendingTileData; 153 private Tile[] mPendingTileData;
148 154
149 /** 155 /**
150 * URL of the most recently removed tile. Used to identify when a tile remov al is confirmed by 156 * URL of the most recently removed tile. Used to identify when a tile remov al is confirmed by
151 * the tile backend. 157 * the tile backend.
152 */ 158 */
153 @Nullable 159 @Nullable
154 private String mPendingRemovalUrl; 160 private String mPendingRemovalUrl;
155 161
162 /**
163 * URL of the most recently added tile. Used to identify when a given tile's insertion is
164 * confirmed by the tile backend. This is relevant when a previously existin g tile is removed,
165 * then the user undoes the action and wants that tile back.
166 */
167 @Nullable
168 private String mPendingInsertionUrl;
169
156 private boolean mHasReceivedData; 170 private boolean mHasReceivedData;
157 171
158 /** 172 /**
159 * @param context Used for initialisation and resolving resources. 173 * @param context Used for initialisation and resolving resources.
160 * @param uiDelegate Delegate used to interact with the rest of the system. 174 * @param uiDelegate Delegate used to interact with the rest of the system.
161 * @param contextMenuManager Used to handle context menu invocations on the tiles. 175 * @param contextMenuManager Used to handle context menu invocations on the tiles.
162 * @param tileGroupDelegate Used for interactions with the Most Visited back end. 176 * @param tileGroupDelegate Used for interactions with the Most Visited back end.
163 * @param observer Will be notified of changes to the tile data. 177 * @param observer Will be notified of changes to the tile data.
164 * @param titleLines The number of text lines to use for each tile title. 178 * @param titleLines The number of text lines to use for each tile title.
165 */ 179 */
(...skipping 23 matching lines...) Expand all
189 mUiDelegate.addDestructionObserver(mOfflineModelObserver); 203 mUiDelegate.addDestructionObserver(mOfflineModelObserver);
190 } else { 204 } else {
191 mOfflineModelObserver = null; 205 mOfflineModelObserver = null;
192 } 206 }
193 } 207 }
194 208
195 @Override 209 @Override
196 public void onMostVisitedURLsAvailable(final String[] titles, final String[] urls, 210 public void onMostVisitedURLsAvailable(final String[] titles, final String[] urls,
197 final String[] whitelistIconPaths, final int[] sources) { 211 final String[] whitelistIconPaths, final int[] sources) {
198 boolean removalCompleted = mPendingRemovalUrl != null; 212 boolean removalCompleted = mPendingRemovalUrl != null;
213 boolean insertionCompleted = mPendingInsertionUrl == null;
214
199 Set<String> addedUrls = new HashSet<>(); 215 Set<String> addedUrls = new HashSet<>();
200 mPendingTileData = new Tile[titles.length]; 216 mPendingTileData = new Tile[titles.length];
201 for (int i = 0; i < titles.length; i++) { 217 for (int i = 0; i < titles.length; i++) {
202 assert urls[i] != null; // We assume everywhere that the url is not null. 218 assert urls[i] != null; // We assume everywhere that the url is not null.
203 219
204 // TODO(dgn): Add UMA to track the cause of https://crbug.com/690926 . Checking this 220 // TODO(dgn): Add UMA to track the cause of https://crbug.com/690926 . Checking this
205 // should not even be necessary as the backend is supposed to send n on dupes URLs. 221 // should not even be necessary as the backend is supposed to send n on dupes URLs.
206 if (addedUrls.contains(urls[i])) { 222 if (addedUrls.contains(urls[i])) {
207 assert false : "Incoming NTP Tiles are not unique. Dupe on " + u rls[i]; 223 assert false : "Incoming NTP Tiles are not unique. Dupe on " + u rls[i];
208 continue; 224 continue;
209 } 225 }
210 226
211 mPendingTileData[i] = 227 mPendingTileData[i] =
212 new Tile(titles[i], urls[i], whitelistIconPaths[i], i, sourc es[i]); 228 new Tile(titles[i], urls[i], whitelistIconPaths[i], i, sourc es[i]);
213 addedUrls.add(urls[i]); 229 addedUrls.add(urls[i]);
214 230
215 if (urls[i].equals(mPendingRemovalUrl)) removalCompleted = false; 231 if (urls[i].equals(mPendingRemovalUrl)) removalCompleted = false;
232 if (urls[i].equals(mPendingInsertionUrl)) insertionCompleted = true;
216 } 233 }
217 234
218 if (removalCompleted) mPendingRemovalUrl = null; 235 boolean expectedChangeCompleted = false;
219 if (!mHasReceivedData || !mUiDelegate.isVisible() || removalCompleted) l oadTiles(); 236 if (mPendingRemovalUrl != null && removalCompleted) {
237 mPendingRemovalUrl = null;
238 expectedChangeCompleted = true;
239 }
240 if (mPendingInsertionUrl != null && insertionCompleted) {
241 mPendingInsertionUrl = null;
242 expectedChangeCompleted = true;
243 }
244
245 if (!mHasReceivedData || !mUiDelegate.isVisible() || expectedChangeCompl eted) loadTiles();
220 } 246 }
221 247
222 @Override 248 @Override
223 public void onIconMadeAvailable(String siteUrl) { 249 public void onIconMadeAvailable(String siteUrl) {
224 Tile tile = getTile(siteUrl); 250 Tile tile = getTile(siteUrl);
225 if (tile == null) return; // The tile might have been removed. 251 if (tile == null) return; // The tile might have been removed.
226 252
227 LargeIconCallback iconCallback = 253 LargeIconCallback iconCallback =
228 new LargeIconCallbackImpl(siteUrl, /* trackLoadTask = */ false); 254 new LargeIconCallbackImpl(siteUrl, /* trackLoadTask = */ false);
229 mUiDelegate.getLargeIconForUrl(siteUrl, mMinIconSize, iconCallback); 255 mUiDelegate.getLargeIconForUrl(siteUrl, mMinIconSize, iconCallback);
(...skipping 199 matching lines...) Expand 10 before | Expand all | Expand 10 after
429 } 455 }
430 456
431 @Override 457 @Override
432 public void removeItem() { 458 public void removeItem() {
433 Tile tile = getTile(mUrl); 459 Tile tile = getTile(mUrl);
434 if (tile == null) return; 460 if (tile == null) return;
435 461
436 // Note: This does not track all the removals, but will track the mo st recent one. If 462 // Note: This does not track all the removals, but will track the mo st recent one. If
437 // that removal is committed, it's good enough for change detection. 463 // that removal is committed, it's good enough for change detection.
438 mPendingRemovalUrl = mUrl; 464 mPendingRemovalUrl = mUrl;
439 mTileGroupDelegate.removeMostVisitedItem(tile); 465 mTileGroupDelegate.removeMostVisitedItem(tile, new RemovalUndoneCall back());
440 } 466 }
441 467
442 @Override 468 @Override
443 public String getUrl() { 469 public String getUrl() {
444 return mUrl; 470 return mUrl;
445 } 471 }
446 472
447 @Override 473 @Override
448 public boolean isItemSupported(@ContextMenuItemId int menuItemId) { 474 public boolean isItemSupported(@ContextMenuItemId int menuItemId) {
449 return true; 475 return true;
450 } 476 }
451 477
452 @Override 478 @Override
453 public void onContextMenuCreated() {} 479 public void onContextMenuCreated() {}
454 480
455 @Override 481 @Override
456 public void onCreateContextMenu( 482 public void onCreateContextMenu(
457 ContextMenu contextMenu, View view, ContextMenuInfo contextMenuI nfo) { 483 ContextMenu contextMenu, View view, ContextMenuInfo contextMenuI nfo) {
458 mContextMenuManager.createContextMenu(contextMenu, view, this); 484 mContextMenuManager.createContextMenu(contextMenu, view, this);
459 } 485 }
460 } 486 }
461 487
488 private class RemovalUndoneCallback extends Callback<String> {
489 @Override
490 public void onResult(String restoredUrl) {
491 mPendingInsertionUrl = restoredUrl;
492 }
493 }
494
462 private class OfflineModelObserver extends SuggestionsOfflineModelObserver<T ile> { 495 private class OfflineModelObserver extends SuggestionsOfflineModelObserver<T ile> {
463 public OfflineModelObserver(OfflinePageBridge bridge) { 496 public OfflineModelObserver(OfflinePageBridge bridge) {
464 super(bridge); 497 super(bridge);
465 } 498 }
466 499
467 @Override 500 @Override
468 public void onSuggestionOfflineIdChanged(Tile suggestion, @Nullable Long id) { 501 public void onSuggestionOfflineIdChanged(Tile suggestion, @Nullable Long id) {
469 // Retrieve a tile from the internal data, to make sure we don't upd ate a stale object. 502 // Retrieve a tile from the internal data, to make sure we don't upd ate a stale object.
470 Tile tile = getTile(suggestion.getUrl()); 503 Tile tile = getTile(suggestion.getUrl());
471 if (tile == null) return; 504 if (tile == null) return;
472 505
473 boolean oldOfflineAvailable = tile.isOfflineAvailable(); 506 boolean oldOfflineAvailable = tile.isOfflineAvailable();
474 tile.setOfflinePageOfflineId(id); 507 tile.setOfflinePageOfflineId(id);
475 508
476 // Only notify to update the view if there will be a visible change. 509 // Only notify to update the view if there will be a visible change.
477 if (oldOfflineAvailable == tile.isOfflineAvailable()) return; 510 if (oldOfflineAvailable == tile.isOfflineAvailable()) return;
478 mObserver.onTileOfflineBadgeVisibilityChanged(tile); 511 mObserver.onTileOfflineBadgeVisibilityChanged(tile);
479 } 512 }
480 513
481 @Override 514 @Override
482 public Iterable<Tile> getOfflinableSuggestions() { 515 public Iterable<Tile> getOfflinableSuggestions() {
483 return Arrays.asList(mTiles); 516 return Arrays.asList(mTiles);
484 } 517 }
485 } 518 }
486 } 519 }
OLDNEW
« no previous file with comments | « no previous file | chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroupDelegateImpl.java » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698