Skip to content

GitLab

T
tangled
Commit 2f08c261 authored by David Maunder's avatar David Maunder Committed by Commit Bot
Browse files
Options

Move Title to CriticalPersistedTabData 

As part of a larger effort to migrate fields in Tab to UserData objects
so Tab doesn't become large and unwieldy.

Bug: 1112364
Change-Id: I838e0542a43d4f8a87685eaa679745cff77ebb28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337576Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Commit-Queue: David Maunder <davidjm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795665}
parent 281c2e3f
Hide whitespace changes
Inline Side-by-side
Showing with 78 additions and 41 deletions
chrome/android/java/src/org/chromium/chrome/browser/tab/TabImpl.java
View file @ 2f08c261
......@@ -173,11 +173,6 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
/** Whether the renderer is currently unresponsive. */
private boolean mIsRendererUnresponsive;
/**
* Title of the ContentViews webpage.
*/
private String mTitle;
/**
* Whether didCommitProvisionalLoadForFrame() hasn't yet been called for the current native page
* (page A). To decrease latency, we show native pages in both loadUrl() and
......@@ -370,9 +365,10 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
@CalledByNative
@Override
// TODO(crbug.com/1113834) migrate getTitle() to CriticalPersistedTabData.from(tab).getTitle()
public String getTitle() {
if (mTitle == null) updateTitle();
return mTitle;
if (CriticalPersistedTabData.from(this).getTitle() == null) updateTitle();
return CriticalPersistedTabData.from(this).getTitle();
}
Context getThemedApplicationContext() {
......@@ -839,7 +835,8 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
CriticalPersistedTabData.from(this).setWebContentsState(state.contentsState);
CriticalPersistedTabData.from(this).setTimestampMillis(state.timestampMillis);
mUrl = new GURL(state.contentsState.getVirtualUrlFromState());
mTitle = state.contentsState.getDisplayTitleFromState();
CriticalPersistedTabData.from(this).setTitle(
state.contentsState.getDisplayTitleFromState());
mLaunchTypeAtCreation = state.tabLaunchTypeAtCreation;
CriticalPersistedTabData.from(this).setRootId(
state.rootId == Tab.INVALID_TAB_ID ? mId : state.rootId);
......@@ -1090,10 +1087,10 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
* @param title Title of the page.
*/
void updateTitle(String title) {
if (TextUtils.equals(mTitle, title)) return;
if (TextUtils.equals(CriticalPersistedTabData.from(this).getTitle(), title)) return;
mIsTabStateDirty = true;
mTitle = title;
CriticalPersistedTabData.from(this).setTitle(title);
notifyPageTitleChanged();
}
......
chrome/android/javatests/src/org/chromium/chrome/browser/MainActivityWithURLTest.java
View file @ 2f08c261
......@@ -47,7 +47,7 @@ public class MainActivityWithURLTest {
testServer.getURL("/chrome/test/data/android/simple.html"));
String expectedTitle = "Activity test page";
TabModel model = mActivityTestRule.getActivity().getCurrentTabModel();
String title = model.getTabAt(model.index()).getTitle();
String title = ChromeTabUtils.getTitleOnUiThread(model.getTabAt(model.index()));
Assert.assertEquals(expectedTitle, title);
} finally {
testServer.stopAndDestroyServer();
......
chrome/android/javatests/src/org/chromium/chrome/browser/TabTest.java
View file @ 2f08c261
......@@ -84,11 +84,13 @@ public class TabTest {
mActivityTestRule.loadUrl("data:text/html;charset=utf-8,<html><head><title>" + oldTitle
+ "</title></head><body/></html>");
Assert.assertEquals("title does not match initial title", oldTitle, mTab.getTitle());
Assert.assertEquals("title does not match initial title", oldTitle,
ChromeTabUtils.getTitleOnUiThread(mTab));
int currentCallCount = mOnTitleUpdatedHelper.getCallCount();
mActivityTestRule.runJavaScriptCodeInCurrentTab("document.title='" + newTitle + "';");
mOnTitleUpdatedHelper.waitForCallback(currentCallCount);
Assert.assertEquals("title does not update", newTitle, mTab.getTitle());
Assert.assertEquals(
"title does not update", newTitle, ChromeTabUtils.getTitleOnUiThread(mTab));
}
/**
......
chrome/android/javatests/src/org/chromium/chrome/browser/UrlSchemeTest.java
View file @ 2f08c261
......@@ -27,9 +27,9 @@ import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.TestContentProvider;
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.net.test.EmbeddedTestServer;
import java.io.File;
......@@ -123,7 +123,8 @@ public class UrlSchemeTest {
// Make sure iframe is really loaded by verifying the title
CriteriaHelper.pollUiThread(() -> {
Criteria.checkThat(mActivityTestRule.getActivity().getActivityTab().getTitle(),
Criteria.checkThat(ChromeTabUtils.getTitleOnUiThread(
mActivityTestRule.getActivity().getActivityTab()),
Matchers.is("iframe loaded"));
});
// Make sure that content provider was asked to provide the content.
......@@ -132,7 +133,8 @@ public class UrlSchemeTest {
// Make sure content access failed by verifying that title is set to fail.
CriteriaHelper.pollUiThread(() -> {
Criteria.checkThat(mActivityTestRule.getActivity().getActivityTab().getTitle(),
Criteria.checkThat(ChromeTabUtils.getTitleOnUiThread(
mActivityTestRule.getActivity().getActivityTab()),
Matchers.is("fail"));
});
}
......@@ -147,14 +149,15 @@ public class UrlSchemeTest {
// Make sure the CORS request fail in the page.
CriteriaHelper.pollUiThread(() -> {
Criteria.checkThat(mActivityTestRule.getActivity().getActivityTab().getTitle(),
Criteria.checkThat(ChromeTabUtils.getTitleOnUiThread(
mActivityTestRule.getActivity().getActivityTab()),
Matchers.not("running"));
});
// Make sure that content provider was asked to provide the content.
ensureResourceRequestCountInContentProviderNotLessThan(resource, 1);
return mActivityTestRule.getActivity().getActivityTab().getTitle();
return ChromeTabUtils.getTitleOnUiThread(mActivityTestRule.getActivity().getActivityTab());
}
@Test
......@@ -199,15 +202,17 @@ public class UrlSchemeTest {
mActivityTestRule.loadUrl(createContentUrl(resource));
CriteriaHelper.pollUiThread(() -> {
Criteria.checkThat(mActivityTestRule.getActivity().getActivityTab().getTitle(),
Criteria.checkThat(ChromeTabUtils.getTitleOnUiThread(
mActivityTestRule.getActivity().getActivityTab()),
Matchers.not("running"));
});
// Make sure that content provider was asked to provide the content.
ensureResourceRequestCountInContentProviderNotLessThan(resource, 1);
Assert.assertEquals(
"exception", mActivityTestRule.getActivity().getActivityTab().getTitle());
Assert.assertEquals("exception",
ChromeTabUtils.getTitleOnUiThread(
mActivityTestRule.getActivity().getActivityTab()));
}
/**
......@@ -244,7 +249,8 @@ public class UrlSchemeTest {
mActivityTestRule.runJavaScriptCodeInCurrentTab(script);
CriteriaHelper.pollUiThread(() -> {
Criteria.checkThat(mActivityTestRule.getActivity().getActivityTab().getTitle(),
Criteria.checkThat(ChromeTabUtils.getTitleOnUiThread(
mActivityTestRule.getActivity().getActivityTab()),
Matchers.is(expectedTitle));
});
ensureResourceRequestCountInContentProviderNotLessThan(resource, expectedLoadCount);
......@@ -282,11 +288,6 @@ public class UrlSchemeTest {
}
}
private String getTitleOnUiThread() {
return TestThreadUtils.runOnUiThreadBlockingNoException(
() -> mActivityTestRule.getActivity().getActivityTab().getTitle());
}
/**
* Test that the browser can be navigated to a file URL.
*/
......@@ -300,7 +301,9 @@ public class UrlSchemeTest {
try {
TestFileUtil.createNewHtmlFile(file, "File", null);
mActivityTestRule.loadUrl("file://" + file.getAbsolutePath());
Assert.assertEquals("File", getTitleOnUiThread());
Assert.assertEquals("File",
ChromeTabUtils.getTitleOnUiThread(
mActivityTestRule.getActivity().getActivityTab()));
} finally {
TestFileUtil.deleteFile(file);
}
......
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabTaskDescriptionHelperTest.java
View file @ 2f08c261
......@@ -202,7 +202,7 @@ public class CustomTabTaskDescriptionHelperTest {
}
private void waitForTitle(Tab tab, String expectedTitle) throws Exception {
if (tab.getTitle().equals(expectedTitle)) return;
if (ChromeTabUtils.getTitleOnUiThread(tab).equals(expectedTitle)) return;
ChromeTabUtils.waitForTitle(tab, expectedTitle);
}
......
chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeTest.java
View file @ 2f08c261
......@@ -41,6 +41,7 @@ import org.chromium.chrome.browser.permissions.PermissionTestRule;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.browser.TabTitleObserver;
import org.chromium.components.browser_ui.notifications.MockNotificationManagerProxy.NotificationEntry;
import org.chromium.components.browser_ui.widget.RoundedIconGenerator;
......@@ -95,7 +96,7 @@ public class NotificationPlatformBridgeTest {
} catch (TimeoutException e) {
// The title is not as expected, this assertion neatly logs what the difference is.
Assert.assertEquals(expectedTitle, tab.getTitle());
Assert.assertEquals(expectedTitle, ChromeTabUtils.getTitleOnUiThread(tab));
}
}
......
chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageAutoFetchTest.java
View file @ 2f08c261
......@@ -238,7 +238,7 @@ public class OfflinePageAutoFetchTest {
// A new tab should open, and it should load the offline page.
pollInstrumentationThread(() -> {
return getCurrentTabModel().getCount() == 2
&& getCurrentTab().getTitle().equals("MyTestPage");
&& ChromeTabUtils.getTitleOnUiThread(getCurrentTab()).equals("MyTestPage");
});
}
......@@ -287,7 +287,7 @@ public class OfflinePageAutoFetchTest {
// No new tab is opened, because the URL of the tab matches the original URL.
return getCurrentTabModel().getCount() == 1
// The title matches the original page, not the 'AlternativeWebServerResponse'.
&& getCurrentTab().getTitle().equals("MyTestPage");
&& ChromeTabUtils.getTitleOnUiThread(getCurrentTab()).equals("MyTestPage");
});
}
......@@ -567,7 +567,7 @@ public class OfflinePageAutoFetchTest {
int tabCount = tabModel.getCount();
Log.d(TAG, "Tab Count: " + tabCount);
for (int i = 0; i < tabCount; ++i) {
String title = tabModel.getTabAt(i).getTitle();
String title = ChromeTabUtils.getTitleOnUiThread(tabModel.getTabAt(i));
String current = tabModel.index() == i ? "*current" : "";
Log.d(TAG, "Tab " + String.valueOf(i) + " '" + title + "' " + current);
}
......
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/suggestions/SwitchToTabTest.java
View file @ 2f08c261
......@@ -38,6 +38,7 @@ import org.chromium.chrome.browser.searchwidget.SearchWidgetProvider;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.OmniboxTestUtils;
import org.chromium.chrome.test.util.WaitForFocusHelper;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
......@@ -100,7 +101,7 @@ public class SwitchToTabTest {
*/
private void typeAndClickMatchingTabMatchSuggestion(Activity activity,
LocationBarLayout locationBarLayout, Tab tab) throws InterruptedException {
typeInOmnibox(activity, tab.getTitle());
typeInOmnibox(activity, ChromeTabUtils.getTitleOnUiThread(tab));
OmniboxTestUtils.waitForOmniboxSuggestions(locationBarLayout);
// waitForOmniboxSuggestions only wait until one suggestion shows up, we need to wait util
......@@ -143,7 +144,8 @@ public class SwitchToTabTest {
for (int i = 0; i < coordinator.getSuggestionCount(); ++i) {
OmniboxSuggestion suggestion = coordinator.getSuggestionAt(i);
if (suggestion != null && suggestion.hasTabMatch()
&& TextUtils.equals(suggestion.getDescription(), tab.getTitle())
&& TextUtils.equals(
suggestion.getDescription(), ChromeTabUtils.getTitleOnUiThread(tab))
&& TextUtils.equals(suggestion.getUrl().getSpec(), tab.getUrl().getSpec())) {
return suggestion;
}
......
chrome/android/javatests/src/org/chromium/chrome/browser/permissions/PermissionTestRule.java
View file @ 2f08c261
......@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.InfoBarTestAnimationListener;
import org.chromium.chrome.test.util.InfoBarUtil;
import org.chromium.components.browser_ui.modaldialog.ModalDialogTestUtils;
......@@ -72,7 +73,8 @@ public class PermissionTestRule extends ChromeActivityTestRule<ChromeActivity> {
@Override
public void onTitleUpdated(Tab tab) {
if (mActivity.getActivityTab().getTitle().equals(mExpectedTitle)) {
if (ChromeTabUtils.getTitleOnUiThread(mActivity.getActivityTab())
.equals(mExpectedTitle)) {
mCallbackHelper.notifyCalled();
}
}
......@@ -89,7 +91,10 @@ public class PermissionTestRule extends ChromeActivityTestRule<ChromeActivity> {
// Update might have already happened, check before waiting for title udpdates.
mExpectedTitle = mPrefix;
if (numUpdates != 0) mExpectedTitle += numUpdates;
if (mActivity.getActivityTab().getTitle().equals(mExpectedTitle)) return;
if (ChromeTabUtils.getTitleOnUiThread(mActivity.getActivityTab())
.equals(mExpectedTitle)) {
return;
}
mCallbackHelper.waitForCallback(0);
}
......
chrome/android/javatests/src/org/chromium/chrome/browser/ssl/CaptivePortalTest.java
View file @ 2f08c261
......@@ -113,8 +113,9 @@ public class CaptivePortalTest {
}
}
.waitForTitleUpdate(INTERSTITIAL_TITLE_UPDATE_TIMEOUT_SECONDS);
Assert.assertEquals(0, tab.getTitle().indexOf(CAPTIVE_PORTAL_INTERSTITIAL_TITLE_PREFIX));
Assert.assertEquals(0,
ChromeTabUtils.getTitleOnUiThread(tab).indexOf(
CAPTIVE_PORTAL_INTERSTITIAL_TITLE_PREFIX));
}
@Test
......
chrome/android/javatests/src/org/chromium/chrome/browser/video/VideoTest.java
View file @ 2f08c261
......@@ -22,6 +22,7 @@ import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.browser.TabTitleObserver;
import org.chromium.content_public.browser.test.util.DOMUtils;
import org.chromium.net.test.EmbeddedTestServer;
......@@ -51,14 +52,14 @@ public class VideoTest {
mActivityTestRule.loadUrl(
testServer.getURL("/chrome/test/data/android/media/video-play.html"));
titleObserver.waitForTitleUpdate(5);
Assert.assertEquals("ready_to_play", tab.getTitle());
Assert.assertEquals("ready_to_play", ChromeTabUtils.getTitleOnUiThread(tab));
titleObserver = new TabTitleObserver(tab, "ended");
DOMUtils.clickNode(tab.getWebContents(), "button1");
// Now the video will play for 5 secs.
// Makes sure that the video ends and title was changed.
titleObserver.waitForTitleUpdate(15);
Assert.assertEquals("ended", tab.getTitle());
Assert.assertEquals("ended", ChromeTabUtils.getTitleOnUiThread(tab));
} finally {
testServer.stopAndDestroyServer();
}
......
chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/state/CriticalPersistedTabData.java
View file @ 2f08c261
......@@ -33,6 +33,10 @@ public class CriticalPersistedTabData extends PersistedTabData {
private static final int UNSPECIFIED_THEME_COLOR = Color.TRANSPARENT;
private static final long INVALID_TIMESTAMP = -1;
/**
* Title of the ContentViews webpage.
*/
private String mTitle;
private int mParentId;
private int mRootId;
private long mTimestampMillis;
......@@ -296,6 +300,20 @@ public class CriticalPersistedTabData extends PersistedTabData {
@Override
public void destroy() {}
/**
* @return title of the {@link Tab}
*/
public String getTitle() {
return mTitle;
}
/**
* @param title of the {@link Tab} to set
*/
public void setTitle(String title) {
mTitle = title;
}
/**
* @return identifier for the {@link Tab}
*/
......
chrome/test/android/javatests/src/org/chromium/chrome/test/util/ChromeTabUtils.java
View file @ 2f08c261
......@@ -48,6 +48,7 @@ import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;
/**
* A utility class that contains methods generic to all Tabs tests.
......@@ -122,6 +123,12 @@ public class ChromeTabUtils {
&& !tab.getWebContents().isLoadingToDifferentDocument();
}
public static String getTitleOnUiThread(Tab tab) {
AtomicReference<String> res = new AtomicReference<>();
TestThreadUtils.runOnUiThreadBlocking(() -> { res.set(tab.getTitle()); });
return res.get();
}
/**
* Waits for the given tab to finish loading the given URL, or, if the given URL is
* null, waits for the current page to load.
......
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