Skip to content

Commit

Permalink
Clean up load logic so that it's easier to understand
Browse files Browse the repository at this point in the history
The logic in Document::DispatchHandleLoadOrLayoutComplete() was there to create different behaviors for main documents vs popups. However, differentiating the main vs popup document is not very transparent in the condition. In addition, this calls HandleLayoutComplete() on the popup which is already a no-op via early return.

Document::DispatchHandleLoadStart() uses a similar confusing condition.

In both cases it's clearer just to call AXObjectCacheImpl methods where we have a useful ::IsPopup() method to help clarify what's happening.

Bug: None
Change-Id: Ie7128ccf6bf6cf2927f416bfb03b15487376eb7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3976391
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1063460}
  • Loading branch information
aleventhal authored and Chromium LUCI CQ committed Oct 25, 2022
1 parent 5607eb6 commit 962d5db
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 21 deletions.
19 changes: 6 additions & 13 deletions third_party/blink/renderer/core/dom/document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3461,20 +3461,13 @@ DocumentParser* Document::ImplicitOpen(
}

void Document::DispatchHandleLoadStart() {
if (AXObjectCache* cache = ExistingAXObjectCache()) {
// Don't fire load start for popup document.
if (this == &AXObjectCacheOwner())
cache->HandleLoadStart(this);
}
if (AXObjectCache* cache = ExistingAXObjectCache())
cache->HandleLoadStart(this);
}

void Document::DispatchHandleLoadOrLayoutComplete() {
if (AXObjectCache* cache = ExistingAXObjectCache()) {
if (this == &AXObjectCacheOwner())
cache->HandleLoadComplete(this);
else
cache->HandleLayoutComplete(this);
}
void Document::DispatchHandleLoadComplete() {
if (AXObjectCache* cache = ExistingAXObjectCache())
cache->HandleLoadComplete(this);
}

HTMLElement* Document::body() const {
Expand Down Expand Up @@ -3713,7 +3706,7 @@ void Document::ImplicitClose() {
load_event_progress_ = kLoadEventCompleted;

if (GetFrame() && GetLayoutView()) {
DispatchHandleLoadOrLayoutComplete();
DispatchHandleLoadComplete();
FontFaceSetDocument::DidLayout(*this);
}

Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/dom/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -1766,7 +1766,7 @@ class CORE_EXPORT Document : public ContainerNode,
bool IsAccessibilityEnabled() const { return !ax_contexts_.empty(); }

void DispatchHandleLoadStart();
void DispatchHandleLoadOrLayoutComplete();
void DispatchHandleLoadComplete();

bool HaveRenderBlockingStylesheetsLoaded() const;
bool HaveRenderBlockingResourcesLoaded() const;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/frame/local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2639,7 +2639,7 @@ void LocalFrame::DidResume() {

// TODO(yuzus): Figure out where these calls should really belong.
GetDocument()->DispatchHandleLoadStart();
GetDocument()->DispatchHandleLoadOrLayoutComplete();
GetDocument()->DispatchHandleLoadComplete();
}

void LocalFrame::MaybeLogAdClickNavigation() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4281,17 +4281,21 @@ void AXObjectCacheImpl::DidHideMenuListPopupWithCleanLayout(Node* menu_list) {

void AXObjectCacheImpl::HandleLoadStart(Document* document) {
SCOPED_DISALLOW_LIFECYCLE_TRANSITION();
MarkAXObjectDirty(Get(document));
DeferTreeUpdate(&AXObjectCacheImpl::EnsurePostNotification, document,
ax::mojom::blink::Event::kLoadStart);
if (!IsPopup(*document)) {
DeferTreeUpdate(&AXObjectCacheImpl::EnsurePostNotification, document,
ax::mojom::blink::Event::kLoadStart);
}
}

void AXObjectCacheImpl::HandleLoadComplete(Document* document) {
SCOPED_DISALLOW_LIFECYCLE_TRANSITION();

AddPermissionStatusListener();
DeferTreeUpdate(&AXObjectCacheImpl::HandleLoadCompleteWithCleanLayout,
document);
// Popups do not need to fire a load complete message.
if (!IsPopup(*document)) {
AddPermissionStatusListener();
DeferTreeUpdate(&AXObjectCacheImpl::HandleLoadCompleteWithCleanLayout,
document);
}
}

void AXObjectCacheImpl::HandleLoadCompleteWithCleanLayout(Node* document_node) {
Expand Down

0 comments on commit 962d5db

Please sign in to comment.