New issue
Advanced search Search tips

Issue 842349 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Make PendingScript the primary class for script scheduling

Project Member Reported by hirosh...@chromium.org, May 11 2018

Issue description

Design doc:
https://docs.google.com/document/d/1y-2n_kR8WdZirOw6uQZGO-wpM5_JKOMBBoVtADN_2EE/edit?usp=sharing

Currently ScriptLoader is needed after PrepareScript(), for script scheduling and execution, but this complicates the semantics of script scheduling, e.g. ScriptRunner schedules scripts via ScriptLoader while HTMLParserScriptRunnner schedules scripts via PendingScript, and sometimes we need to get ScriptLoader from PendingScript and vice versa.

This issue makes PendingScript the primary class for script scheduling and execution, and removes ScriptLoader dependency after PrepareScript().
 
Cc: vogelheim@chromium.org
+vogelheim@ FYI. This issue will make ScriptRunner directly touch PendingScript to schedule script streaming.
Project Member

Comment 2 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98e5bb29d4d53b135a89488879fccbd14caa89d5

commit 98e5bb29d4d53b135a89488879fccbd14caa89d5
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Wed May 16 01:17:23 2018

Redirect ScriptLoader::IsReady() to PendingScript::IsReady() in unit tests

This CL makes MockScriptLoader::IsReady() just to call
PendingScript::IsReady(), and mocks PendingScript::IsReady() instead.

This is to remove ScriptLoader dependency from IsReady(), and
in a subsequent CL [1] ScriptLoader::IsReady() is replaced with
PendingScript::IsReady()

This requires MockScriptLoader to have a corresponding
MockPendingScript reference, and thus this CL adds
MockScriptLoader::mock_pending_script_.

MockScriptLoader::mock_pending_script_if_script_is_async_
will be removed in [1].

[1] https://chromium-review.googlesource.com/1053220

Bug:  842349 
Change-Id: Ia61b3ccbeb175bece4add5c0a4f2a542118f0f26
Reviewed-on: https://chromium-review.googlesource.com/1041072
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558913}
[modify] https://crrev.com/98e5bb29d4d53b135a89488879fccbd14caa89d5/third_party/blink/renderer/core/script/script_runner_test.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bbbbe120b65f3207c26b31c55d2a955e3dcae8de

commit bbbbe120b65f3207c26b31c55d2a955e3dcae8de
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu May 17 04:56:23 2018

Set IsReady() more systematically in unit tests

This CL sets IsReady() false at MockPendingScript construction and
sets it true in ScriptRunnerTest::NotifyScriptReady().
This removes most of IsReady() EXPECT_CALL()s.

Bug:  842349 
Change-Id: I633f4fa17add8f149efc068b8b2a4573db2a6c53
Reviewed-on: https://chromium-review.googlesource.com/1041131
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559423}
[modify] https://crrev.com/bbbbe120b65f3207c26b31c55d2a955e3dcae8de/third_party/blink/renderer/core/script/script_runner_test.cc

Project Member

Comment 4 by bugdroid1@chromium.org, May 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/734f483b48a232d55103145c92f5a6ac4050e329

commit 734f483b48a232d55103145c92f5a6ac4050e329
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri May 18 01:35:04 2018

Introduce ScriptLoader::GetPendingScriptIfControlledByScriptRunner()

ScriptRunner calls ScriptLoader methods, e.g.
GetPendingScriptIfScriptOfAsyncScript(), IsReady(), Execute(),
which can be eventually replaced with direct calls to PendingScript.

To incrementally move those things from ScriptLoader to PendingScript,
this CL introduces
ScriptLoader::GetPendingScriptIfControlledByScriptRunner().

During the incremental move,
calls to ScriptLoader from ScriptRunner are replaced with
GetPendingScriptIfControlledByScriptRunner() + calls to PendingScript,
and then replaced with direct calls to PendingScript.

Once the incremental move is completely done,
GetPendingScriptIfControlledByScriptRunner() can be removed.

This CL also removes ScriptLoader::IsReady() by replacing it
with GetPendingScriptIfControlledByScriptRunner()->IsReady().

No behavior changes.

Bug:  842349 
Change-Id: Idb8c0231c432e99af6fd3dd1092a47667ebcc54f
Reviewed-on: https://chromium-review.googlesource.com/1053220
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559771}
[modify] https://crrev.com/734f483b48a232d55103145c92f5a6ac4050e329/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/734f483b48a232d55103145c92f5a6ac4050e329/third_party/blink/renderer/core/script/script_loader.h
[modify] https://crrev.com/734f483b48a232d55103145c92f5a6ac4050e329/third_party/blink/renderer/core/script/script_runner.cc
[modify] https://crrev.com/734f483b48a232d55103145c92f5a6ac4050e329/third_party/blink/renderer/core/script/script_runner_test.cc

Project Member

Comment 5 by bugdroid1@chromium.org, May 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/462a606f77d9318980a92181abe96104fd0277f6

commit 462a606f77d9318980a92181abe96104fd0277f6
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri May 18 20:58:30 2018

Remove ScriptLoader::IsScriptTypeSupported()

ScriptLoader::IsScriptTypeSupported() doesn't require ScriptLoader.
This CL replaces it with a static method
IsValidScriptTypeAndLanguage() and removes one
ScriptElementBase::Loader() reference, as a part of reducing
unnecessary references to ScriptLoader.

No behavior changes.

Bug:  842349 
Change-Id: I4f6d7df0652c8c5b948a50c6b123aed1be4d8287
Reviewed-on: https://chromium-review.googlesource.com/1041265
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560028}
[modify] https://crrev.com/462a606f77d9318980a92181abe96104fd0277f6/third_party/blink/renderer/core/html/html_script_element.cc
[modify] https://crrev.com/462a606f77d9318980a92181abe96104fd0277f6/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/462a606f77d9318980a92181abe96104fd0277f6/third_party/blink/renderer/core/script/script_loader.h

Project Member

Comment 6 by bugdroid1@chromium.org, May 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f3818923d2b7815524c013a177bbea281e44e5b3

commit f3818923d2b7815524c013a177bbea281e44e5b3
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri May 18 21:18:18 2018

Move ScriptLoader::have_fire_load_ to SVGScriptElement

To reduce ScriptLoader dependency after PrepareScript(),
this CL moves ScriptLoader::have_fire_load_ to SVGScriptElement,
where the flag is actually checked.

After this CL, ScriptLoader::DispatchErrorEvent() and
ScriptLoader::DispatchLoadEvent() can be removed with
ScriptElementBase's counterparts.

No behavior changes.

Bug:  842349 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Id87c92e6d25d04af358122f4f699b91b9c826be1
Reviewed-on: https://chromium-review.googlesource.com/1052831
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560040}
[modify] https://crrev.com/f3818923d2b7815524c013a177bbea281e44e5b3/third_party/blink/renderer/core/html/html_script_element.cc
[modify] https://crrev.com/f3818923d2b7815524c013a177bbea281e44e5b3/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/f3818923d2b7815524c013a177bbea281e44e5b3/third_party/blink/renderer/core/script/script_loader.h
[modify] https://crrev.com/f3818923d2b7815524c013a177bbea281e44e5b3/third_party/blink/renderer/core/svg/svg_script_element.cc
[modify] https://crrev.com/f3818923d2b7815524c013a177bbea281e44e5b3/third_party/blink/renderer/core/svg/svg_script_element.h

Project Member

Comment 7 by bugdroid1@chromium.org, May 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4e5b525e912092d121cf42cce60fd5c4581421e6

commit 4e5b525e912092d121cf42cce60fd5c4581421e6
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Mon May 21 23:32:19 2018

Introduce ScriptSchedulingType and remove ScriptLoader::async_exec_type_

This CL
- Introduces ScriptSchedulingType to clarify scheduling concept, and
- Moves ScriptLoader::async_exec_type_ to
  PendingScript::scheduling_type_,
  to remove ScriptLoader dependency from script scheduling.

No behavior changes.

Bug:  842349 
Change-Id: I8dc5c05a8d51090274e9f4634ed389f3d299e901
Reviewed-on: https://chromium-review.googlesource.com/1053303
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560404}
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/BUILD.gn
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/html_parser_script_runner.cc
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/pending_script.cc
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/pending_script.h
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/script_loader.h
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/script_runner.cc
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/script_runner.h
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/script_runner_test.cc
[add] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/script_scheduling_type.h
[modify] https://crrev.com/4e5b525e912092d121cf42cce60fd5c4581421e6/third_party/blink/renderer/core/script/xml_parser_script_runner.cc

Project Member

Comment 8 by bugdroid1@chromium.org, May 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b276a021a873c69b5920246ba3f827fbce759f0f

commit b276a021a873c69b5920246ba3f827fbce759f0f
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Mon May 21 23:59:01 2018

Set MockPendingScript's element

To enable getting ScriptLoader from PendingScript by
GetElement()->Loader().

Bug:  842349 
Change-Id: I35bec7df7a5e4bda99e587e872882ed2d0a032b6
Reviewed-on: https://chromium-review.googlesource.com/1053369
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560417}
[modify] https://crrev.com/b276a021a873c69b5920246ba3f827fbce759f0f/third_party/blink/renderer/core/script/script_runner_test.cc

Project Member

Comment 9 by bugdroid1@chromium.org, May 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bbea96225dec1eedec20fea69ba30185ec39281e

commit bbea96225dec1eedec20fea69ba30185ec39281e
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Tue May 22 01:09:36 2018

Change ScriptLoader::resource_keep_alive_ clear timing

To remove ScriptLoader::Execute(), this CL clears
ScriptLoader::resource_keep_alive_ in TakePendingScript() for
ScriptRunner-controlled scripts.

The behavior and the lifetime of ScriptResource is not changed,
because the ScriptResource is anyway kept alive until script execution,
because ClassicPendingScript (which is alive until script execution)
has a reference to ScriptResource until script execution.

Bug:  842349 
Change-Id: Ia839cbd00e1b19a2ee915565516ce92e0571530c
Reviewed-on: https://chromium-review.googlesource.com/1053995
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560440}
[modify] https://crrev.com/bbea96225dec1eedec20fea69ba30185ec39281e/third_party/blink/renderer/core/script/script_loader.cc

Project Member

Comment 10 by bugdroid1@chromium.org, May 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c42053157d119ccfa815ac25a81bb28d9e04691

commit 2c42053157d119ccfa815ac25a81bb28d9e04691
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Tue May 22 01:37:01 2018

Move ScriptLoader::original_document_ to PendingScript

To reduce dependency to ScriptLoader after PrepareScript(),
this CL moves original_document_ to PendingScript.

Because PendingScript is created within PrepareScript(),
|original_document_| before and after this CL
refer to the same Document.

No behavior changes.

Bug:  842349 
Change-Id: Iaf185a25153e05cceee1057b3e4165e210c5c6db
Reviewed-on: https://chromium-review.googlesource.com/1053158
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560449}
[modify] https://crrev.com/2c42053157d119ccfa815ac25a81bb28d9e04691/third_party/blink/renderer/core/script/pending_script.cc
[modify] https://crrev.com/2c42053157d119ccfa815ac25a81bb28d9e04691/third_party/blink/renderer/core/script/pending_script.h
[modify] https://crrev.com/2c42053157d119ccfa815ac25a81bb28d9e04691/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/2c42053157d119ccfa815ac25a81bb28d9e04691/third_party/blink/renderer/core/script/script_loader.h

Project Member

Comment 11 by bugdroid1@chromium.org, May 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f2d377f38f425f87cc68b1ce3d0521cf470b5401

commit f2d377f38f425f87cc68b1ce3d0521cf470b5401
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Tue May 22 19:09:26 2018

Remove ScriptLoader::start_line_number_

ScriptLoader and related classes use two start line numbers:
- ScriptLoader::PrepareScript()'s script_start_position argument
  == PendingScript::StartingPosition()
  == ScriptSourceCode::StartPosition().
- ScriptLoader::start_line_number_ which is set to
  GetScriptableDocumentParser()->LineNumber() at ScriptLoader construction, and
  which is used for CSP error reporting, and

This CL merges the latter into the former, because the former is more broadly used.

This is for consistency and code simplification.

Bug:  842349 
Change-Id: I7fb945d7bf6ce40ac670234637e816a368c17c84
Reviewed-on: https://chromium-review.googlesource.com/1053159
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560719}
[modify] https://crrev.com/f2d377f38f425f87cc68b1ce3d0521cf470b5401/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/f2d377f38f425f87cc68b1ce3d0521cf470b5401/third_party/blink/renderer/core/script/script_loader.h

Project Member

Comment 12 by bugdroid1@chromium.org, May 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20c0462ab63d8137a72fb7ff080686313415c51a

commit 20c0462ab63d8137a72fb7ff080686313415c51a
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Tue May 22 19:59:24 2018

Use IsInDocumentWrite() at the time of PrepareScript() instead of element creation

ScriptLoader and related classes use two document.write()-related flags:
- Document::IsInDocumentWrite() at the time of PrepareScript():
  directly checked within PrepareScript() and used for multiple purposes.
- Document::IsInDocumentWrite() at the time of <script> element creation:
  plumbed from HTML parser to ScriptLoader::created_during_document_write_
  and used for DocumentParserTiming.

This CL merges the latter into the former:
- Moves ScriptLoader::created_during_document_write_ to
  PendingScript::created_during_document_write_ which is set to IsInDocumentWrite()
  at the time of PendingScript creation, which is within PrepareScript(), and
- Removes the plumbing of the flag from the HTML parser to ScriptLoader.

Thus this CL simplifies the code by reducing plumbing through ScriptLoader constructor.

Bug:  842349 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I2b17dd098bb134a59b9c1ba2937c74bfa46b4c0f
Reviewed-on: https://chromium-review.googlesource.com/1053163
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560739}
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/dom/create_element_flags.h
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/html/html_script_element.cc
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/html/parser/html_construction_site.cc
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/script/pending_script.cc
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/script/pending_script.h
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/script/script_element_base.cc
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/script/script_element_base.h
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/script/script_loader.h
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/script/script_runner_test.cc
[modify] https://crrev.com/20c0462ab63d8137a72fb7ff080686313415c51a/third_party/blink/renderer/core/svg/svg_script_element.cc

Project Member

Comment 13 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa3ce76198cdacbeef020de8c0b34799cac97405

commit aa3ce76198cdacbeef020de8c0b34799cac97405
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Wed May 23 01:17:33 2018

Use PendingScript in ScriptRunner

This CL replaces the most of ScriptLoader references in
ScriptRunner with PendingScript.

ScriptLoader is still used
- When the script is evaluated,
  because ScriptLoader::Execute() is still in ScriptLoader.
  ScriptLoader is obtained by PendingScript::GetElement()->Loader()
  here, but this will be removed by [1] once
  ScriptLoader::Execute() is removed.
- When the script element is moved between documents, because
  we have to get PendingScript via the
  Element->ScriptLoader->PendingScript path.
  This is done by
  GetPendingScriptIfControlledByScriptRunnerForCrossDocMove()
  in MovePendingScript(), which will be the last use of
  GetPendingScriptIfControlledByScriptRunner()
  that will remain until [2] that is not planned to land soon.

No behavior changes.

[1] https://chromium-review.googlesource.com/1054553
[2] https://chromium-review.googlesource.com/1041143

Bug:  842349 
Change-Id: Idc36b00c8c25fe8e24ff60e980608d8b324fec5e
Reviewed-on: https://chromium-review.googlesource.com/1053354
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560874}
[modify] https://crrev.com/aa3ce76198cdacbeef020de8c0b34799cac97405/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/aa3ce76198cdacbeef020de8c0b34799cac97405/third_party/blink/renderer/core/script/script_loader.h
[modify] https://crrev.com/aa3ce76198cdacbeef020de8c0b34799cac97405/third_party/blink/renderer/core/script/script_runner.cc
[modify] https://crrev.com/aa3ce76198cdacbeef020de8c0b34799cac97405/third_party/blink/renderer/core/script/script_runner.h
[modify] https://crrev.com/aa3ce76198cdacbeef020de8c0b34799cac97405/third_party/blink/renderer/core/script/script_runner_test.cc

Project Member

Comment 14 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a33419400a318625885a4e4ec73896f1f476cbc

commit 0a33419400a318625885a4e4ec73896f1f476cbc
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Wed May 23 16:30:51 2018

Move ScriptLoader::ExecuteScriptBlock() to PendingScript

This CL moves ExecuteScriptBlock() to PendingScript, and
removes dependency to ScriptLoader (via
PendingScript::GetElement()->Loader()) outside ScriptRunner.

ScriptRunner still needs ScriptLoader for Execute(), which
will be removed by
https://chromium-review.googlesource.com/1054553/.

No behavior changes.

Bug:  842349 
Change-Id: I903a33f6fe54b0f60ac87921cd37eb687a5681c1
Reviewed-on: https://chromium-review.googlesource.com/1053555
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561117}
[modify] https://crrev.com/0a33419400a318625885a4e4ec73896f1f476cbc/third_party/blink/renderer/core/script/html_parser_script_runner.cc
[modify] https://crrev.com/0a33419400a318625885a4e4ec73896f1f476cbc/third_party/blink/renderer/core/script/pending_script.cc
[modify] https://crrev.com/0a33419400a318625885a4e4ec73896f1f476cbc/third_party/blink/renderer/core/script/pending_script.h
[modify] https://crrev.com/0a33419400a318625885a4e4ec73896f1f476cbc/third_party/blink/renderer/core/script/script_element_base.h
[modify] https://crrev.com/0a33419400a318625885a4e4ec73896f1f476cbc/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/0a33419400a318625885a4e4ec73896f1f476cbc/third_party/blink/renderer/core/script/script_loader.h
[modify] https://crrev.com/0a33419400a318625885a4e4ec73896f1f476cbc/third_party/blink/renderer/core/script/xml_parser_script_runner.cc

Project Member

Comment 16 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/849d5f907251e39a27732322423a784b83215c16

commit 849d5f907251e39a27732322423a784b83215c16
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu May 24 01:28:01 2018

Remove ScriptLoader::Execute()

This CL replaces ScriptLoader::Execute() with
PendingScript::ExecuteScriptBlock().

To do this, this CL clears ScriptLoader::pending_script_ in
ScriptLoader::PendingScriptFinished(), i.e. slightly before
ScriptLoader::Execute().

Bug:  842349 
Change-Id: Ia010d370ce1e139808b39e66ad4557f9151e2a47
Reviewed-on: https://chromium-review.googlesource.com/1054553
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561351}
[modify] https://crrev.com/849d5f907251e39a27732322423a784b83215c16/third_party/blink/renderer/core/script/pending_script.h
[modify] https://crrev.com/849d5f907251e39a27732322423a784b83215c16/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/849d5f907251e39a27732322423a784b83215c16/third_party/blink/renderer/core/script/script_loader.h
[modify] https://crrev.com/849d5f907251e39a27732322423a784b83215c16/third_party/blink/renderer/core/script/script_runner.cc
[modify] https://crrev.com/849d5f907251e39a27732322423a784b83215c16/third_party/blink/renderer/core/script/script_runner_test.cc

Project Member

Comment 17 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb39631d14c78a76176a5629c8eefed8accf4b69

commit eb39631d14c78a76176a5629c8eefed8accf4b69
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu May 24 03:09:28 2018

Remove ScriptLoader from script_runner_test.cc

This CL removes MockScriptLoader definition,
migrates MockScriptLoader::Create*() to MockPendingScript, and
makes ScriptLoader final.

Other changes to test bodies are mechanical:
- Replaced script_loader with pending_script,
- Replaced ScriptLoader with PendingScript,
- Remove "->GetPendingScriptIfControlledByScriptRunner()",

Bug:  842349 
Change-Id: I515890efeaffcb0f8dc8c9dad1102591974e8d67
Reviewed-on: https://chromium-review.googlesource.com/1056399
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561372}
[modify] https://crrev.com/eb39631d14c78a76176a5629c8eefed8accf4b69/third_party/blink/renderer/core/script/script_runner_test.cc

Project Member

Comment 18 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b74c8557b018b2af3bd49cc1e64cd4a3c0b9cd84

commit b74c8557b018b2af3bd49cc1e64cd4a3c0b9cd84
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu May 24 03:42:11 2018

Forbid accessing ScriptLoader from PendingScript

This CL makes ScriptElementBase::Loader() protected, and thus
after this CL PendingScript no longer has access to ScriptLoader,
making separation between PendingScript and ScriptLoader more strict.

XML/HTMLParserScriptRunner previously get ScriptLoader via
ScriptElementBase, but after this CL they get ScriptLoader
directly via ScriptLoaderFromElement().

No behavior changes.

Bug:  842349 
Change-Id: I5104c0817669e871a68b8d8286d037e3e23622cc
Reviewed-on: https://chromium-review.googlesource.com/1054855
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561379}
[modify] https://crrev.com/b74c8557b018b2af3bd49cc1e64cd4a3c0b9cd84/third_party/blink/renderer/core/script/html_parser_script_runner.cc
[modify] https://crrev.com/b74c8557b018b2af3bd49cc1e64cd4a3c0b9cd84/third_party/blink/renderer/core/script/script_element_base.cc
[modify] https://crrev.com/b74c8557b018b2af3bd49cc1e64cd4a3c0b9cd84/third_party/blink/renderer/core/script/script_element_base.h
[modify] https://crrev.com/b74c8557b018b2af3bd49cc1e64cd4a3c0b9cd84/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/b74c8557b018b2af3bd49cc1e64cd4a3c0b9cd84/third_party/blink/renderer/core/script/xml_parser_script_runner.cc

Project Member

Comment 19 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/78dd26a211196944f095a20fefcd3ed2dafebe6d

commit 78dd26a211196944f095a20fefcd3ed2dafebe6d
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu May 24 18:21:46 2018

Remove ScriptLoader::DispatchLoad/ErrorEvent()

No behavior changes.

Bug:  842349 
Change-Id: Iebd91d4d0f98441fd613d7bc98e26e06767915cf
Reviewed-on: https://chromium-review.googlesource.com/1056415
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561555}
[modify] https://crrev.com/78dd26a211196944f095a20fefcd3ed2dafebe6d/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/78dd26a211196944f095a20fefcd3ed2dafebe6d/third_party/blink/renderer/core/script/script_loader.h

Project Member

Comment 20 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b300cfa77d3e972f56c67c1f27f51bc0af4a658

commit 4b300cfa77d3e972f56c67c1f27f51bc0af4a658
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu May 24 20:47:38 2018

Remove ScriptLoader::GetPendingScriptIfControlledByScriptRunner()

As refactoring CLs for  Issue 842349  landed, this is no longer used.

Bug:  842349 
Change-Id: I94af990ce81e1d67ded2913e18c4b92e327fa856
Reviewed-on: https://chromium-review.googlesource.com/1069688
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561619}
[modify] https://crrev.com/4b300cfa77d3e972f56c67c1f27f51bc0af4a658/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/4b300cfa77d3e972f56c67c1f27f51bc0af4a658/third_party/blink/renderer/core/script/script_loader.h

Status: Fixed (was: Started)
Done.

Sign in to add a comment