Commit 169f6091 authored by Theresa Wellington's avatar Theresa Wellington Committed by Commit Bot

Fix AppMenuPropertiesDelegateImpl NPE on BookmarkBridge

A previous CL refactorted AppMenuPropertiesDelegateImpl to use an
ObservableSupplier for obtaining the BookmarkBridge rather than a setter
in the AppMenuPropertiesDelegate interface. When a callback is
registered with the ObservableSupplier, a call to the callback is posted
if the supplied object is already available. This changed the timing of
when the BookmarkBridge is supplied, allowing the app menu to be opened
before the BookmarkBridge is supplied.

This CL attempts to get the BookmarkBridge from the supplier directly
when #updateBookmarkMenuItem is called if the BookmarkBridge is null and
adds another null check in case the bridge is still not available.

BUG=1027222

Change-Id: I8d8bbeb812286c8c18baf0ae65da11bd48386d04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1934448Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Theresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718696}
parent 08f761b8
......@@ -455,7 +455,20 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
* @param currentTab Current tab being displayed.
*/
protected void updateBookmarkMenuItem(MenuItem bookmarkMenuItem, Tab currentTab) {
bookmarkMenuItem.setEnabled(mBookmarkBridge.isEditBookmarksEnabled());
// If this method is called before the {@link #mBookmarkBridgeSupplierCallback} has been
// called, try to retrieve the bridge directly from the supplier.
if (mBookmarkBridge == null && mBookmarkBridgeSupplier != null) {
mBookmarkBridge = mBookmarkBridgeSupplier.get();
}
if (mBookmarkBridge == null) {
// If the BookmarkBridge still isn't available, assume the bookmark menu item is not
// editable.
bookmarkMenuItem.setEnabled(false);
} else {
bookmarkMenuItem.setEnabled(mBookmarkBridge.isEditBookmarksEnabled());
}
if (BookmarkBridge.hasBookmarkIdForTab(currentTab)) {
bookmarkMenuItem.setIcon(R.drawable.btn_star_filled);
bookmarkMenuItem.setChecked(true);
......@@ -474,7 +487,7 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
* @param currentTab Current tab being displayed.
*/
protected void updateRequestDesktopSiteMenuItem(
Menu menu, Tab currentTab, boolean canShowRequestDekstopSite) {
Menu menu, Tab currentTab, boolean canShowRequestDesktopSite) {
MenuItem requestMenuRow = menu.findItem(R.id.request_desktop_site_row_menu_id);
MenuItem requestMenuLabel = menu.findItem(R.id.request_desktop_site_id);
MenuItem requestMenuCheck = menu.findItem(R.id.request_desktop_site_check_id);
......@@ -486,7 +499,7 @@ public class AppMenuPropertiesDelegateImpl implements AppMenuPropertiesDelegate
// Also hide request desktop site on Reader Mode.
boolean isDistilledPage = DomDistillerUrlUtils.isDistilledPage(url);
boolean itemVisible = canShowRequestDekstopSite
boolean itemVisible = canShowRequestDesktopSite
&& (!isChromeScheme || currentTab.isNativePage()) && !isDistilledPage;
requestMenuRow.setVisible(itemVisible);
if (!itemVisible) return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment