Skip to content

Commit

Permalink
Revert "CDP: fix Page.addScriptToEvaluateOnNewDocument in iframes"
Browse files Browse the repository at this point in the history
This reverts commit f9fe901204382c43273aa46d0cd3d7382d014f43.

Reason for revert: None results are generated on jetstream benchmark. More details in crbug/1263067

Original change's description:
> CDP: fix Page.addScriptToEvaluateOnNewDocument in iframes
>
> When iframe cancels the initial navigation by document.open,
> we did not create isolated worlds and did not evaluate scripts
> on new document.
>
> Ideally, we would force a context and evaluate/create worlds
> in the initial empty document, because it could be actually
> become the real document in multiple circumstances (e.g. window.stop).
> Unfortunately, this breaks assumptions in random places
> like GuestView (see Patchset 4).
>
> For now, only cover document.open path.
>
> Change-Id: I1651196526c23dec1f8c25dd79926e0ae5ff8887
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3236032
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#934240}

Change-Id: Ibb16f4c3ea8ff4662e5cdb4bed04160dbd71d1ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3248283
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: John Chen <johnchen@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Wenbin Zhang <wenbinzhang@google.com>
Cr-Commit-Position: refs/heads/main@{#936027}
NOKEYCHECK=True
GitOrigin-RevId: a88cb8a4d819217aead181948e2a6d9a40d555c4
  • Loading branch information
Wenbin Zhang authored and copybara-github committed Oct 28, 2021
1 parent b57db58 commit 1b7de78
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 61 deletions.
12 changes: 2 additions & 10 deletions blink/renderer/core/inspector/inspector_page_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -931,10 +931,6 @@ scoped_refptr<DOMWrapperWorld> InspectorPageAgent::EnsureDOMWrapperWorld(
}

void InspectorPageAgent::DidClearDocumentOfWindowObject(LocalFrame* frame) {
EvaluateScriptsOnNewDocument(frame);
}

void InspectorPageAgent::EvaluateScriptsOnNewDocument(LocalFrame* frame) {
if (!GetFrontend())
return;
Vector<WTF::String> keys = scripts_to_evaluate_on_load_.Keys();
Expand Down Expand Up @@ -1033,15 +1029,11 @@ void InspectorPageAgent::DidRestoreFromBackForwardCache(LocalFrame* frame) {
protocol::Page::NavigationTypeEnum::BackForwardCacheRestore);
}

void InspectorPageAgent::DidOpenDocument(
LocalFrame* frame,
DocumentLoader* loader,
bool is_empty_document_opened_first_time) {
void InspectorPageAgent::DidOpenDocument(LocalFrame* frame,
DocumentLoader* loader) {
GetFrontend()->documentOpened(BuildObjectForFrame(loader->GetFrame()));
LifecycleEvent(frame, loader, "init",
base::TimeTicks::Now().since_origin().InSecondsF());
if (is_empty_document_opened_first_time)
EvaluateScriptsOnNewDocument(frame);
}

void InspectorPageAgent::FrameAttachedToParent(LocalFrame* frame) {
Expand Down
5 changes: 1 addition & 4 deletions blink/renderer/core/inspector/inspector_page_agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,7 @@ class CORE_EXPORT InspectorPageAgent final
void LoadEventFired(LocalFrame*);
void WillCommitLoad(LocalFrame*, DocumentLoader*);
void DidRestoreFromBackForwardCache(LocalFrame*);
void DidOpenDocument(LocalFrame*,
DocumentLoader*,
bool is_empty_document_opened_first_time);
void DidOpenDocument(LocalFrame*, DocumentLoader*);
void FrameAttachedToParent(LocalFrame*);
void FrameDetachedFromParent(LocalFrame*, FrameDetachType);
void FrameStartedLoading(LocalFrame*);
Expand Down Expand Up @@ -256,7 +254,6 @@ class CORE_EXPORT InspectorPageAgent final
LocalFrame* frame,
const String& world_name,
bool grant_universal_access);
void EvaluateScriptsOnNewDocument(LocalFrame*);

static KURL UrlWithoutFragment(const KURL&);

Expand Down
7 changes: 2 additions & 5 deletions blink/renderer/core/loader/frame_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,8 @@ void FrameLoader::DispatchUnloadEvent(
}

void FrameLoader::DidExplicitOpen() {
bool is_empty_document_opened_first_time =
empty_document_status_ == EmptyDocumentStatus::kOnlyEmpty;
probe::DidOpenDocument(frame_, GetDocumentLoader(),
is_empty_document_opened_first_time);
if (is_empty_document_opened_first_time)
probe::DidOpenDocument(frame_, GetDocumentLoader());
if (empty_document_status_ == EmptyDocumentStatus::kOnlyEmpty)
empty_document_status_ = EmptyDocumentStatus::kOnlyEmptyButExplicitlyOpened;

// Only model a document.open() as part of a navigation if its parent is not
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/core/probe/core_probes.pidl
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ interface CoreProbes {
void DidCommitLoad([Keep] LocalFrame*, DocumentLoader*);
void DidNavigateWithinDocument([Keep] LocalFrame*);
void DidRestoreFromBackForwardCache([Keep] LocalFrame*);
void DidOpenDocument([Keep] LocalFrame*, DocumentLoader*, bool is_empty_document_opened_first_time);
void DidOpenDocument([Keep] LocalFrame*, DocumentLoader*);
void FrameDocumentUpdated([Keep] LocalFrame*);
void FrameOwnerContentUpdated([Keep] LocalFrame*, HTMLFrameOwnerElement*);
void FrameStartedLoading([Keep] LocalFrame*);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,14 @@
Tests that Page.addScriptToEvaluateOnNewDocument is executed in the given world
Adding scripts
<main world> in main frame
<main world> in main frame
world#0 in main frame
world#0
message from 0
world#1 in main frame
world#1
message from 1
world#2 in main frame
world#2
message from 2
world#3 in main frame
world#3
message from 3
world#4 in main frame
world#4
message from 4
added iframe
<main world> in subframe
world#0 in subframe
message from 0
world#1 in subframe
message from 1
world#2 in subframe
message from 2
world#3 in subframe
message from 3
world#4 in subframe
message from 4
written to iframe <head></head><body>hello</body>
Removing scripts
Navigating cross-process
<main world> in main frame

Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
(async function(testRunner) {
const {page, session, dp} = await testRunner.startBlank(
'Tests that Page.addScriptToEvaluateOnNewDocument is executed in the given world');
dp.Runtime.enable();
dp.Page.enable();
const mainFrameId = (await dp.Page.getFrameTree()).result.frameTree.frame.id;

dp.Runtime.enable();
const scriptIds = [];
dp.Runtime.onConsoleAPICalled(msg => testRunner.log(msg.params.args[0].value));
const logContextCreationCallback = msg => {
const suffix = mainFrameId === msg.params.context.auxData.frameId ? 'main frame' : 'subframe';
testRunner.log(`${msg.params.context.name || '<main world>'} in ${suffix}`);
if (msg.params.context.name.includes('world'))
testRunner.log(msg.params.context.name);
};
dp.Runtime.onExecutionContextCreated(logContextCreationCallback);

Expand All @@ -22,28 +21,15 @@

await session.navigate('../resources/blank.html');

await session.evaluate(`
const iframe = document.createElement('iframe');
// The url does not matter since we document.open immediately below.
// It must not be about:blank though, to avoid sync commit.
iframe.src = 'http://google.com';
document.body.appendChild(iframe);
console.log('added iframe');
iframe.contentDocument.open();
iframe.contentDocument.write('hello');
iframe.contentDocument.close();
console.log('written to iframe ' + iframe.contentDocument.documentElement.innerHTML);
`);

testRunner.log('Removing scripts');
for (let identifier of scriptIds) {
const response = await dp.Page.removeScriptToEvaluateOnNewDocument({identifier});
if (!response.result)
testRunner.log('Failed script removal');
}

testRunner.log('Navigating cross-process');
await session.navigate('http://127.0.0.1:8000/inspector-protocol/resources/empty.html');
dp.Runtime.offExecutionContextCreated(logContextCreationCallback);
await session.navigate('../resources/blank.html');

testRunner.completeTest();
})

0 comments on commit 1b7de78

Please sign in to comment.