0

[DevTools][MultiInstance] Support new tab in another window on Android

(test stolen from https://chromium-review.googlesource.com/c/chromium/src/+/6236167)

Bug: 401269942, 382183406
Change-Id: Ib57d90015acbc38ee9603d399f612083cca1efa4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6352169
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Auto-Submit: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1432579}
This commit is contained in:
Michael Thiessen
2025-03-14 01:27:22 -07:00
committed by Chromium LUCI CQ
parent 97d4730ee7
commit 08b5aad019
25 changed files with 138 additions and 45 deletions

@ -4,12 +4,17 @@
package org.chromium.chrome.browser.tabmodel;
import android.app.Activity;
import android.content.Intent;
import android.os.Bundle;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import org.chromium.base.ContextUtils;
import org.chromium.base.MathUtils;
import org.chromium.base.ObserverList;
import org.chromium.base.TraceEvent;
@ -17,9 +22,13 @@ import org.chromium.base.metrics.RecordUserAction;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.WarmupManager;
import org.chromium.chrome.browser.app.tab_activity_glue.ReparentingTask;
import org.chromium.chrome.browser.flags.ActivityType;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.homepage.HomepageManager;
import org.chromium.chrome.browser.multiwindow.MultiInstanceManager;
import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabCreationState;
@ -960,6 +969,40 @@ public class TabModelImpl extends TabModelJniBridge {
.createNewTab(loadUrlParams, tabLaunchType, persistParentage ? parent : null);
}
@Override
public Tab createNewTabForDevTools(GURL url, boolean newWindow) {
LoadUrlParams loadParams = new LoadUrlParams(url);
@TabLaunchType int launchType = TabLaunchType.FROM_CHROME_UI;
if (!newWindow
|| MultiWindowUtils.getInstanceCount() >= MultiWindowUtils.getMaxInstances()) {
return getTabCreator(/* incognito= */ false).createNewTab(loadParams, launchType, null);
}
// Creating a new window is asynchronous on Android, so create a background tab that we can
// return immediately and reparent it into a new window.
WarmupManager warmupManager = WarmupManager.getInstance();
Tab parentTab = TabModelUtils.getCurrentTab(this);
Profile profile = parentTab.getProfile();
warmupManager.createRegularSpareTab(profile);
Tab tab = warmupManager.takeSpareTab(profile, /* initiallyHidden= */ false, launchType);
tab.loadUrl(loadParams);
MultiInstanceManager.onMultiInstanceModeStarted();
Intent intent =
MultiWindowUtils.createNewWindowIntent(
parentTab.getContext(),
MultiWindowUtils.INVALID_INSTANCE_ID,
/* preferNew= */ true,
/* openAdjacently= */ true,
/* addTrustedIntentExtras= */ true);
Activity activity = ContextUtils.activityFromContext(parentTab.getContext());
Bundle options = MultiWindowUtils.getOpenInOtherWindowActivityOptions(activity);
ReparentingTask.from(tab).begin(activity, intent, options, null);
return tab;
}
@Override
public int getCount() {
return mTabs.size();

@ -15,8 +15,6 @@ import org.jni_zero.NativeMethods;
import org.chromium.chrome.browser.flags.ActivityType;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.WebContents;
import org.chromium.content_public.common.ResourceRequestBody;
import org.chromium.url.GURL;
@ -177,13 +175,12 @@ public abstract class TabModelJniBridge implements TabModelInternal {
/**
* Creates a Tab with the given WebContents for DevTools.
*
* @param url URL to show.
* @param newWindow Whether to open the new tab in a new window.
*/
@CalledByNative
protected Tab createNewTabForDevTools(GURL url) {
return getTabCreator(/* incognito= */ false)
.createNewTab(new LoadUrlParams(url), TabLaunchType.FROM_CHROME_UI, null);
}
protected abstract Tab createNewTabForDevTools(GURL url, boolean newWindow);
/** Returns whether supplied Tab instance has been grouped together with other Tabs. */
@CalledByNative

@ -276,7 +276,8 @@ DevToolsAgentHost::List DevToolsManagerDelegateAndroid::RemoteDebuggingTargets(
scoped_refptr<DevToolsAgentHost>
DevToolsManagerDelegateAndroid::CreateNewTarget(
const GURL& url,
DevToolsManagerDelegate::TargetType target_type) {
DevToolsManagerDelegate::TargetType target_type,
bool new_window) {
if (TabModelList::models().empty())
return nullptr;
@ -284,9 +285,11 @@ DevToolsManagerDelegateAndroid::CreateNewTarget(
if (!tab_model)
return nullptr;
WebContents* web_contents = tab_model->CreateNewTabForDevTools(url);
if (!web_contents)
WebContents* web_contents =
tab_model->CreateNewTabForDevTools(url, new_window);
if (!web_contents) {
return nullptr;
}
MarkCreatedByDevTools(*web_contents);
return target_type == DevToolsManagerDelegate::kTab

@ -29,7 +29,8 @@ class DevToolsManagerDelegateAndroid : public content::DevToolsManagerDelegate {
TargetType target_type) override;
scoped_refptr<content::DevToolsAgentHost> CreateNewTarget(
const GURL& url,
TargetType target_type) override;
TargetType target_type,
bool new_window) override;
bool IsBrowserTargetDiscoverable() override;
};

@ -432,7 +432,8 @@ void ChromeDevToolsManagerDelegate::ClientDetached(
scoped_refptr<DevToolsAgentHost> ChromeDevToolsManagerDelegate::CreateNewTarget(
const GURL& url,
DevToolsManagerDelegate::TargetType target_type) {
DevToolsManagerDelegate::TargetType target_type,
bool new_window) {
NavigateParams params(ProfileManager::GetLastUsedProfile(), url,
ui::PAGE_TRANSITION_AUTO_TOPLEVEL);
params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;

@ -97,7 +97,8 @@ class ChromeDevToolsManagerDelegate : public content::DevToolsManagerDelegate {
content::DevToolsAgentHostClientChannel* channel) override;
scoped_refptr<content::DevToolsAgentHost> CreateNewTarget(
const GURL& url,
TargetType target_type) override;
TargetType target_type,
bool new_window) override;
bool HasBundledFrontendResources() override;
void DevicesAvailable(

@ -170,7 +170,8 @@ class ExtensionPlatformBrowserTest::TestTabModel : public TabModel {
bool select) override {}
void HandlePopupNavigation(TabAndroid* parent,
NavigateParams* params) override {}
content::WebContents* CreateNewTabForDevTools(const GURL& url) override {
content::WebContents* CreateNewTabForDevTools(const GURL& url,
bool new_window) override {
return nullptr;
}
bool IsSessionRestoreInProgress() const override { return false; }

@ -74,7 +74,8 @@ std::optional<size_t> OpenNewTab(const GURL& url) {
size_t tab_index = 0;
#if BUILDFLAG(IS_ANDROID)
tab_index = GetActiveTabModel()->GetTabCount();
web_contents = GetActiveTabModel()->CreateNewTabForDevTools(url);
web_contents =
GetActiveTabModel()->CreateNewTabForDevTools(url, /*new_window=*/false);
#else
TabStripModel* tab_strip = GetBrowserOrDie()->tab_strip_model();
tab_index = tab_strip->count();

@ -206,7 +206,8 @@ class TabModel {
// Used by Developer Tools to create a new tab with a given URL.
// Replaces CreateTabForTesting.
virtual content::WebContents* CreateNewTabForDevTools(const GURL& url) = 0;
virtual content::WebContents* CreateNewTabForDevTools(const GURL& url,
bool new_window) = 0;
// Return true if we are currently restoring sessions asynchronously.
virtual bool IsSessionRestoreInProgress() const = 0;

@ -158,14 +158,15 @@ void TabModelJniBridge::CloseTabAt(int index) {
Java_TabModelJniBridge_closeTabAt(env, java_object_.get(env), index);
}
WebContents* TabModelJniBridge::CreateNewTabForDevTools(const GURL& url) {
WebContents* TabModelJniBridge::CreateNewTabForDevTools(const GURL& url,
bool new_window) {
// TODO(dfalcantara): Change the Java side so that it creates and returns the
// WebContents, which we can load the URL on and return.
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj =
Java_TabModelJniBridge_createNewTabForDevTools(
env, java_object_.get(env),
url::GURLAndroid::FromNativeGURL(env, url));
url::GURLAndroid::FromNativeGURL(env, url), new_window);
if (obj.is_null()) {
VLOG(0) << "Failed to create java tab";
return NULL;

@ -60,7 +60,8 @@ class TabModelJniBridge : public TabModel {
void HandlePopupNavigation(TabAndroid* parent,
NavigateParams* params) override;
content::WebContents* CreateNewTabForDevTools(const GURL& url) override;
content::WebContents* CreateNewTabForDevTools(const GURL& url,
bool new_window) override;
// Return true if we are currently restoring sessions asynchronously.
bool IsSessionRestoreInProgress() const override;

@ -40,7 +40,8 @@ void TestTabModel::CreateTab(TabAndroid* parent,
void TestTabModel::HandlePopupNavigation(TabAndroid* parent,
NavigateParams* params) {}
content::WebContents* TestTabModel::CreateNewTabForDevTools(const GURL& url) {
content::WebContents* TestTabModel::CreateNewTabForDevTools(const GURL& url,
bool new_window) {
return nullptr;
}

@ -25,7 +25,8 @@ class TestTabModel : public TabModel {
bool select) override;
void HandlePopupNavigation(TabAndroid* parent,
NavigateParams* params) override;
content::WebContents* CreateNewTabForDevTools(const GURL& url) override;
content::WebContents* CreateNewTabForDevTools(const GURL& url,
bool new_window) override;
bool IsSessionRestoreInProgress() const override;
bool IsActiveModel() const override;

@ -6157,6 +6157,31 @@ class ChromeDriverAndroidTest(ChromeDriverBaseTest):
self.assertGreaterEqual(size[0], 20)
self.assertGreaterEqual(size[1], 20)
def testAndroidOpenNewWindow(self):
self._driver = self.CreateDriver()
size = self._driver.GetWindowRect()
old_window_handle = self._driver.GetCurrentWindowHandle()
# window1 = self._driver.SendCommandAndGetResult(
# 'Browser.getWindowForTarget', {'targetId': old_window_handle})
new_window = self._driver.NewWindow(window_type='window')
self._driver.SwitchToWindow(new_window['handle'])
self.assertTrue(
self.WaitForCondition(
lambda: self._driver.GetCurrentWindowHandle() !=
old_window_handle))
new_window_handle = self._driver.GetCurrentWindowHandle()
self.assertNotEqual(None, new_window_handle)
self.assertNotEqual(old_window_handle, new_window_handle)
# TODO(crbug.com/6236167): Until Browser.getWindowForTarget is supported
# on Android, it's not possible to assert window IDs of two tabs are
# different.
# window2 = self._driver.SendCommandAndGetResult(
# 'Browser.getWindowForTarget', {'targetId': new_window_handle})
# Verify that the second tab target is indeed a different window.
# self.assertNotEqual(window1['windowId'], window2['windowId'])
class ChromeDownloadDirTest(ChromeDriverBaseTest):
def RespondWithCsvFile(self, request):

@ -643,9 +643,11 @@ void DevToolsHttpHandler::OnJsonRequest(
url = GURL(url::kAboutBlankURL);
// TODO(dsv): Remove for "for_tab" support once DevTools Frontend
// no longer needs it for e2e tests
scoped_refptr<DevToolsAgentHost> agent_host = delegate_->CreateNewTarget(
url, for_tab ? DevToolsManagerDelegate::kTab
: DevToolsManagerDelegate::kFrame);
scoped_refptr<DevToolsAgentHost> agent_host =
delegate_->CreateNewTarget(url,
for_tab ? DevToolsManagerDelegate::kTab
: DevToolsManagerDelegate::kFrame,
/*new_window=*/false);
if (!agent_host) {
SendJson(connection_id, net::HTTP_INTERNAL_SERVER_ERROR, std::nullopt,
"Could not create new page");

@ -155,7 +155,7 @@ class MockDevToolsManagerDelegate : public DevToolsManagerDelegate {
MockDevToolsManagerDelegate() { last_instance = this; }
MOCK_METHOD(scoped_refptr<DevToolsAgentHost>,
CreateNewTarget,
(const GURL& url, TargetType target_type),
(const GURL& url, TargetType target_type, bool new_window),
(override));
MOCK_METHOD(DevToolsAgentHost::List,
RemoteDebuggingTargets,
@ -308,7 +308,8 @@ TEST_F(DevToolsHttpHandlerTest, MutatingActionsiRequireSafeVerb) {
EXPECT_CALL(
*MockDevToolsManagerDelegate::last_instance,
CreateNewTarget(GURL("about:blank"), DevToolsManagerDelegate::kFrame))
CreateNewTarget(GURL("about:blank"), DevToolsManagerDelegate::kFrame,
/*new_window=*/false))
.WillOnce(Return(base::MakeRefCounted<MockDevToolsAgentHost>()));
request = request_context->CreateRequest(
@ -338,7 +339,8 @@ TEST_F(DevToolsHttpHandlerTest, TestJsonNew) {
EXPECT_CALL(
*MockDevToolsManagerDelegate::last_instance,
CreateNewTarget(GURL("about:blank"), DevToolsManagerDelegate::kFrame));
CreateNewTarget(GURL("about:blank"), DevToolsManagerDelegate::kFrame,
/*new_window=*/false));
GURL url(base::StringPrintf("http://127.0.0.1:%d/json/new", port));
auto request_context = net::CreateTestURLRequestContextBuilder()->Build();
auto request = request_context->CreateRequest(
@ -348,9 +350,10 @@ TEST_F(DevToolsHttpHandlerTest, TestJsonNew) {
delegate.RunUntilComplete();
EXPECT_GE(delegate.request_status(), 0);
EXPECT_CALL(*MockDevToolsManagerDelegate::last_instance,
CreateNewTarget(GURL("http://example.com"),
DevToolsManagerDelegate::kFrame));
EXPECT_CALL(
*MockDevToolsManagerDelegate::last_instance,
CreateNewTarget(GURL("http://example.com"),
DevToolsManagerDelegate::kFrame, /*new_window=*/false));
url = GURL(base::StringPrintf(
"http://127.0.0.1:%d/json/new?%s", port,
base::EscapeQueryParamValue("http://example.com", true).c_str()));
@ -361,9 +364,10 @@ TEST_F(DevToolsHttpHandlerTest, TestJsonNew) {
delegate.RunUntilComplete();
EXPECT_GE(delegate.request_status(), 0);
EXPECT_CALL(*MockDevToolsManagerDelegate::last_instance,
CreateNewTarget(GURL("http://example.com"),
DevToolsManagerDelegate::kTab));
EXPECT_CALL(
*MockDevToolsManagerDelegate::last_instance,
CreateNewTarget(GURL("http://example.com"), DevToolsManagerDelegate::kTab,
/*new_window=*/false));
url = GURL(base::StringPrintf(
"http://127.0.0.1:%d/json/new?%s&for_tab", port,
base::EscapeQueryParamValue("http://example.com", true).c_str()));

@ -1240,7 +1240,7 @@ Response TargetHandler::CreateTarget(
? content::DevToolsManagerDelegate::kTab
: content::DevToolsManagerDelegate::kFrame;
scoped_refptr<content::DevToolsAgentHost> agent_host =
delegate->CreateNewTarget(gurl, target_type);
delegate->CreateNewTarget(gurl, target_type, new_window.value_or(false));
if (!agent_host)
return Response::ServerError("Not supported");
*out_target_id = agent_host->GetId();

@ -37,7 +37,8 @@ DevToolsAgentHost::List DevToolsManagerDelegate::RemoteDebuggingTargets(
scoped_refptr<DevToolsAgentHost> DevToolsManagerDelegate::CreateNewTarget(
const GURL& url,
DevToolsManagerDelegate::TargetType target_type) {
DevToolsManagerDelegate::TargetType target_type,
bool new_window) {
return nullptr;
}

@ -56,9 +56,12 @@ class CONTENT_EXPORT DevToolsManagerDelegate {
virtual DevToolsAgentHost::List RemoteDebuggingTargets(TargetType target_type);
// Creates new inspectable target given the |url|.
virtual scoped_refptr<DevToolsAgentHost> CreateNewTarget(
const GURL& url,
TargetType target_type);
// |new_window| is currently only used on Android - Desktop platforms handle
// window creation elsewhere. Note that there is also a limit to the number of
// windows that may be opened on Android, and this parameter may be ignored if
// new windows cannot be opened.
virtual scoped_refptr<DevToolsAgentHost>
CreateNewTarget(const GURL& url, TargetType target_type, bool new_window);
// Get all live browser contexts created by CreateBrowserContext() method.
virtual std::vector<BrowserContext*> GetBrowserContexts();

@ -219,7 +219,8 @@ void ShellDevToolsManagerDelegate::HandleCommand(
scoped_refptr<DevToolsAgentHost> ShellDevToolsManagerDelegate::CreateNewTarget(
const GURL& url,
content::DevToolsManagerDelegate::TargetType target_type) {
content::DevToolsManagerDelegate::TargetType target_type,
bool new_window) {
Shell* shell = Shell::CreateNewWindow(browser_context_, url, nullptr,
Shell::GetShellDefaultSize());
return target_type == content::DevToolsManagerDelegate::kTab

@ -33,9 +33,9 @@ class ShellDevToolsManagerDelegate : public DevToolsManagerDelegate {
void HandleCommand(content::DevToolsAgentHostClientChannel* channel,
base::span<const uint8_t> message,
NotHandledCallback callback) override;
scoped_refptr<DevToolsAgentHost> CreateNewTarget(
const GURL& url,
TargetType target_type) override;
scoped_refptr<DevToolsAgentHost> CreateNewTarget(const GURL& url,
TargetType target_type,
bool new_window) override;
std::string GetDiscoveryPageHTML() override;
bool HasBundledFrontendResources() override;
void ClientAttached(

@ -36,7 +36,8 @@ void HeadlessDevToolsManagerDelegate::HandleCommand(
scoped_refptr<content::DevToolsAgentHost>
HeadlessDevToolsManagerDelegate::CreateNewTarget(
const GURL& url,
content::DevToolsManagerDelegate::TargetType target_type) {
content::DevToolsManagerDelegate::TargetType target_type,
bool new_window) {
if (!browser_)
return nullptr;

@ -33,7 +33,8 @@ class HeadlessDevToolsManagerDelegate
NotHandledCallback callback) override;
scoped_refptr<content::DevToolsAgentHost> CreateNewTarget(
const GURL& url,
TargetType target_type) override;
TargetType target_type,
bool new_window) override;
bool HasBundledFrontendResources() override;
void ClientAttached(
content::DevToolsAgentHostClientChannel* channel) override;

@ -24,7 +24,8 @@ content::BrowserContext* DevToolsManagerDelegate::GetDefaultBrowserContext() {
scoped_refptr<content::DevToolsAgentHost>
DevToolsManagerDelegate::CreateNewTarget(
const GURL& url,
content::DevToolsManagerDelegate::TargetType target_type) {
content::DevToolsManagerDelegate::TargetType target_type,
bool new_window) {
content::WebContents* web_content =
create_content_window_func_.Run(browser_context_.get(), url);
return target_type == content::DevToolsManagerDelegate::kTab

@ -33,7 +33,8 @@ class DevToolsManagerDelegate : public content::DevToolsManagerDelegate {
content::BrowserContext* GetDefaultBrowserContext() override;
scoped_refptr<content::DevToolsAgentHost> CreateNewTarget(
const GURL& url,
TargetType target_type) override;
TargetType target_type,
bool new_window) override;
std::string GetDiscoveryPageHTML() override;
bool HasBundledFrontendResources() override;