0

Bulk rewrite of NavigationHandle::IsInMainFrame callers to IsInPrimaryMainFrame in extensions/

With MPArch (Multiple Page Architecture), we are introducing the ability
for a single WebContents to have multiple pages and therefore multiple
main frames. Based on an audit of existing uses of IsInMainFrame, we
expect most callers are only interested in the "primary" main frame.
Accordingly, we perform a bulk rewrite of IsInMainFrame calls to
IsInPrimaryMainFrame in order to preserve the existing semantics.

The methodology is as follows:
Run the following script:
find extensions/ -type f \( -name "*.cc" -o -name "*.h" -o -name "*.mm" \) -a \! \( -name "*unittest.cc" -o -name "*browsertest.cc" \) -exec sed --in-place --file IsInMainFrame-rewrite-script {} \;
where IsInMainFrame-rewrite-script contains:
/[Ii]sInMainFrame(/ i\
\/\/ TODO(https:\/\/crbug.com\/1218946): Comment goes here.
s/\([Ii]\)sInMainFrame(/\1sInPrimaryMainFrame(/g

Patchset 1 contains the result of the purely mechanical part of this
change. Patchset 2 reverts changes to navigation throttles which are
not part of this rewrite. Patchset 3 reverts conversions that are
noticeably incorrect.

Bug: 1218946
Change-Id: I53fb7011a02b80f00bcef65396559ac20b0d075e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2995069
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#902501}
This commit is contained in:
Kevin McNee
2021-07-16 17:25:19 +00:00
committed by Chromium LUCI CQ
parent cd8df4cf6f
commit 029c081de6
3 changed files with 16 additions and 4 deletions
extensions/browser/guest_view

@ -248,7 +248,10 @@ WebContents* ExtensionOptionsGuest::CreateCustomWebContents(
void ExtensionOptionsGuest::DidFinishNavigation(
content::NavigationHandle* navigation_handle) {
if (!navigation_handle->IsInMainFrame() ||
// TODO(https://crbug.com/1218946): With MPArch there may be multiple main
// frames. This caller was converted automatically to the primary main frame
// to preserve its semantics. Follow up to confirm correctness.
if (!navigation_handle->IsInPrimaryMainFrame() ||
!navigation_handle->HasCommitted() || !attached()) {
return;
}

@ -228,7 +228,10 @@ void MimeHandlerStreamManager::EmbedderObserver::ReadyToCommitNavigation(
void MimeHandlerStreamManager::EmbedderObserver::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
// If the top level frame is navigating away, clean up the stream.
if (navigation_handle->IsInMainFrame() &&
// TODO(https://crbug.com/1218946): With MPArch there may be multiple main
// frames. This caller was converted automatically to the primary main frame
// to preserve its semantics. Follow up to confirm correctness.
if (navigation_handle->IsInPrimaryMainFrame() &&
!navigation_handle->IsSameDocument()) {
AbortStream();
}

@ -922,7 +922,10 @@ void WebViewGuest::DidFinishNavigation(
return;
}
if (navigation_handle->IsInMainFrame() && pending_zoom_factor_) {
// TODO(https://crbug.com/1218946): With MPArch there may be multiple main
// frames. This caller was converted automatically to the primary main frame
// to preserve its semantics. Follow up to confirm correctness.
if (navigation_handle->IsInPrimaryMainFrame() && pending_zoom_factor_) {
// Handle a pending zoom if one exists.
SetZoom(pending_zoom_factor_);
pending_zoom_factor_ = 0.0;
@ -969,7 +972,10 @@ void WebViewGuest::DocumentOnLoadCompletedInMainFrame(
void WebViewGuest::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
WebViewGuest* opener = GetOpener();
if (opener && navigation_handle->IsInMainFrame()) {
// TODO(https://crbug.com/1218946): With MPArch there may be multiple main
// frames. This caller was converted automatically to the primary main frame
// to preserve its semantics. Follow up to confirm correctness.
if (opener && navigation_handle->IsInPrimaryMainFrame()) {
auto it = opener->pending_new_windows_.find(this);
if (it != opener->pending_new_windows_.end()) {
NewWindowInfo& info = it->second;