Clean up ownership semantics for NavigateParams.
This CL is a refactor with no intended behavior change.
Previously, NavigateParams was a copy-able class that sometimes passed
ownership of its member target_contents to the Navigate() method. Furthermore,
target_contents was overloaded to have different semantics before, during, and
after the call to Navigate().
This CL makes NavigateParams a move-only class. It has the input parameter
|contents_to_insert| which explicitly passes ownership [with clear semantics] to
the Navigate() method. It has another input parameter |switch_to_singleton_tab|
to indicate that a tab switch should occur, but no ownership is passed. Finaly,
it has an output parameter |navigated_or_inserted_contents| which does not
convey ownership.
With these new semantics, the helper class ScopedTargetContentsOwner is no
longer necessary. Furthermore, the Navigate() method now uses an internal,
temporary variable |contents_to_navigate_or_insert| rather than overloading the
semantics of NavigateParams.target_contents.
This CL creates a new struct PrerenderManager::Params to function as both input
and output parameters to PrerenderManager::MaybeUsePrerenderedPage, rather than
overloading the NavigateParam struct to have different semantics.
Change-Id: Ic94284b9f6d2a0305375127858020271f7a97601
Bug: 832879
Reviewed-on: https://chromium-review.googlesource.com/1028458
Reviewed-by: Nico Weber <[email protected]>
Reviewed-by: Sergey Volk <[email protected]>
Reviewed-by: Ken Rockot <[email protected]>
Reviewed-by: Egor Pasko <[email protected]>
Commit-Queue: Erik Chen <[email protected]>
Cr-Commit-Position: refs/heads/master@{#554128}diff --git a/chrome/browser/extensions/extension_tab_util.cc b/chrome/browser/extensions/extension_tab_util.cc
index fae948e..fbfb3d7b 100644
--- a/chrome/browser/extensions/extension_tab_util.cc
+++ b/chrome/browser/extensions/extension_tab_util.cc
@@ -251,8 +251,8 @@
// The tab may have been created in a different window, so make sure we look
// at the right tab strip.
tab_strip = navigate_params.browser->tab_strip_model();
- int new_index =
- tab_strip->GetIndexOfWebContents(navigate_params.target_contents);
+ int new_index = tab_strip->GetIndexOfWebContents(
+ navigate_params.navigated_or_inserted_contents);
if (opener) {
// Only set the opener if the opener tab is in the same tab strip as the
// new tab.
@@ -261,12 +261,12 @@
}
if (active)
- navigate_params.target_contents->SetInitialFocus();
+ navigate_params.navigated_or_inserted_contents->SetInitialFocus();
// Return data about the newly created tab.
- return ExtensionTabUtil::CreateTabObject(navigate_params.target_contents,
- kScrubTab, function->extension(),
- tab_strip, new_index)
+ return ExtensionTabUtil::CreateTabObject(
+ navigate_params.navigated_or_inserted_contents, kScrubTab,
+ function->extension(), tab_strip, new_index)
->ToValue()
.release();
}
@@ -620,7 +620,7 @@
return false;
}
-void ExtensionTabUtil::CreateTab(WebContents* web_contents,
+void ExtensionTabUtil::CreateTab(std::unique_ptr<WebContents> web_contents,
const std::string& extension_id,
WindowOpenDisposition disposition,
const gfx::Rect& initial_rect,
@@ -633,7 +633,7 @@
Browser::CreateParams params = Browser::CreateParams(profile, user_gesture);
browser = new Browser(params);
}
- NavigateParams params(browser, web_contents);
+ NavigateParams params(browser, std::move(web_contents));
// The extension_app_id parameter ends up as app_name in the Browser
// which causes the Browser to return true for is_app(). This affects
@@ -731,7 +731,7 @@
params.path_behavior = open_in_tab ? NavigateParams::RESPECT
: NavigateParams::IGNORE_AND_NAVIGATE;
params.url = url_to_navigate;
- ShowSingletonTabOverwritingNTP(browser, params);
+ ShowSingletonTabOverwritingNTP(browser, std::move(params));
return true;
}
