New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 689702 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Clean up html parser now that it is single threaded

Project Member Reported by csharrison@chromium.org, Feb 7 2017

Issue description

Some areas to clean up (at least partially):
-BackgroundHTMLInputStream
-CompactHTMLToken{Test}
-HTMLParserThread
-HTMLParserThreadTest
-TokenizedChunkQueue
-MediaValuesCached
-BackgroundHTMLParser
-PreloadRequest
 
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 16 2017

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

commit 138a364d84e54ac2b6cc494a7245627745054341
Author: csharrison <csharrison@chromium.org>
Date: Thu Feb 16 17:06:27 2017

Revert of Put the BackgroundHTMLParser on oilpan heap (patchset #6 id:100001 of https://codereview.chromium.org/2683653004/ )

Reason for revert:
Causing frequent crashes: crbug.com/692999

Original issue's description:
> Put the BackgroundHTMLParser on oilpan heap
>
> This patch puts BackgroundHTMLParser on the oilpan heap, as well as
> moving asynchrony logic to the BackgroundHTMLParser.
>
> BUG=689702
>
> Review-Url: https://codereview.chromium.org/2683653004
> Cr-Commit-Position: refs/heads/master@{#450559}
> Committed: https://chromium.googlesource.com/chromium/src/+/7fe94f0a7f3809fd93916c526d067dcb7f2b4ce3

TBR=yoav@yoav.ws,kouhei@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=689702,692999

Review-Url: https://codereview.chromium.org/2695633007
Cr-Commit-Position: refs/heads/master@{#450997}

[modify] https://crrev.com/138a364d84e54ac2b6cc494a7245627745054341/third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.cpp
[modify] https://crrev.com/138a364d84e54ac2b6cc494a7245627745054341/third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.h
[modify] https://crrev.com/138a364d84e54ac2b6cc494a7245627745054341/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp
[modify] https://crrev.com/138a364d84e54ac2b6cc494a7245627745054341/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.h

Project Member

Comment 4 by sheriffbot@chromium.org, Feb 16 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 5 by tkent@chromium.org, Feb 20 2018

Labels: -Type-Bug -Hotlist-Recharge-Cold Type-Task
Status: Available (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 27 2018

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

commit 91155378db70515df1dda8c650237a38549b93fa
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Apr 27 05:19:34 2018

HTMLDocumentParser: Remove TokenizedChunkQueue

TokenizedChunkQueue is a deprecated optimization where it reduced number
of tasks being posted from BackgroundHTMLParserThread to the main thread.

Now that we actually run the BackgroundHTMLParser on the main thread,
we no longer need this optimization, we can queue the speculation anytime
we want.

Bonus: Now that we have move semantics, just use CompactHTMLToken directly, not via unique_ptr.

Bug: 689702
Change-Id: I0a219b0baba79a4293d8e8bd09300837c1c6bf4c
Reviewed-on: https://chromium-review.googlesource.com/1029552
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Yoav Weiss <yoav@yoav.ws>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554315}
[modify] https://crrev.com/91155378db70515df1dda8c650237a38549b93fa/third_party/blink/renderer/core/html/parser/BUILD.gn
[modify] https://crrev.com/91155378db70515df1dda8c650237a38549b93fa/third_party/blink/renderer/core/html/parser/background_html_parser.cc
[modify] https://crrev.com/91155378db70515df1dda8c650237a38549b93fa/third_party/blink/renderer/core/html/parser/background_html_parser.h
[modify] https://crrev.com/91155378db70515df1dda8c650237a38549b93fa/third_party/blink/renderer/core/html/parser/html_document_parser.cc
[modify] https://crrev.com/91155378db70515df1dda8c650237a38549b93fa/third_party/blink/renderer/core/html/parser/html_document_parser.h
[delete] https://crrev.com/e082f999c8b2e9e43b2220ff6b4481d1ccc12c74/third_party/blink/renderer/core/html/parser/tokenized_chunk_queue.cc
[delete] https://crrev.com/e082f999c8b2e9e43b2220ff6b4481d1ccc12c74/third_party/blink/renderer/core/html/parser/tokenized_chunk_queue.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 27 2018

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

commit dc224d2824dbadc6a09e9663a569010566cb0871
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Apr 27 09:18:18 2018

BackgroundHTMLParser: Make token limits unconfigurable

I couldn't find any occurrence in tests that attempt to dynamically reconfigure this.
This CL just makes them use constants to reduce complexity.

Bug: 689702
Change-Id: I431aaec674a439d82499a1b4099d4de64e75f5c1
Reviewed-on: https://chromium-review.googlesource.com/1032313
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554343}
[modify] https://crrev.com/dc224d2824dbadc6a09e9663a569010566cb0871/third_party/blink/renderer/core/frame/settings.json5
[modify] https://crrev.com/dc224d2824dbadc6a09e9663a569010566cb0871/third_party/blink/renderer/core/html/parser/background_html_parser.cc
[modify] https://crrev.com/dc224d2824dbadc6a09e9663a569010566cb0871/third_party/blink/renderer/core/html/parser/background_html_parser.h
[modify] https://crrev.com/dc224d2824dbadc6a09e9663a569010566cb0871/third_party/blink/renderer/core/html/parser/html_document_parser.cc

Project Member

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

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

commit 9f63db45c1a5bf808e12b2c5a4228b57b9f9f434
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Tue May 01 06:58:48 2018

Re-introduce nullchk before calling EnqueueTokenizedChunk

This CL reintroduces null check of WeakPtr parser_ which was
accidentally removed in 91155378db70515df1dda8c650237a38549b93fa

I have manually tested that this fixes the clusterfuzz test cases.
To be confirmed via clusterfuzz bots.

Bug: 837948,  837893 ,  837693 , 689702
Change-Id: I8f149803e24fb949a36fbeee96f17f80a62f6ea0
Reviewed-on: https://chromium-review.googlesource.com/1036885
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555012}
[modify] https://crrev.com/9f63db45c1a5bf808e12b2c5a4228b57b9f9f434/third_party/blink/renderer/core/html/parser/background_html_parser.cc

Project Member

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

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

commit 05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc
Author: csharrison <csharrison@chromium.org>
Date: Mon May 21 00:59:21 2018

Remove some unused thread-safe html/parser code

Bug: 689702
Change-Id: I3d536827dc572de1cc315226aefbded5b53aee03
Reviewed-on: https://chromium-review.googlesource.com/1066894
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560215}
[modify] https://crrev.com/05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc/third_party/blink/renderer/core/html/parser/background_html_parser.cc
[modify] https://crrev.com/05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc/third_party/blink/renderer/core/html/parser/background_html_parser.h
[modify] https://crrev.com/05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc/third_party/blink/renderer/core/html/parser/compact_html_token.cc
[modify] https://crrev.com/05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc/third_party/blink/renderer/core/html/parser/compact_html_token.h
[modify] https://crrev.com/05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc/third_party/blink/renderer/core/html/parser/preload_request.cc
[modify] https://crrev.com/05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc/third_party/blink/renderer/core/html/parser/preload_request.h
[modify] https://crrev.com/05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc/third_party/blink/renderer/core/html/parser/xss_auditor_delegate.cc
[modify] https://crrev.com/05b51a1e81c411a96c2f709bc23a4d1d24ab6cdc/third_party/blink/renderer/core/html/parser/xss_auditor_delegate.h

Project Member

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

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

commit 1dc33c218b41eb6812fe9585b360dacbc533dbeb
Author: csharrison <csharrison@chromium.org>
Date: Mon May 21 01:14:11 2018

Remove thread-safe copying in PreloadRequest

Bug: 689702
Change-Id: I2d42bdc23d384462955a70f53c6462298df30658
Reviewed-on: https://chromium-review.googlesource.com/1067102
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560216}
[modify] https://crrev.com/1dc33c218b41eb6812fe9585b360dacbc533dbeb/third_party/blink/renderer/core/html/parser/preload_request.h

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 10

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

commit 2932b8bf7174dbd4ca26a7a8c162962824b15233
Author: Kent Tamura <tkent@chromium.org>
Date: Tue Jul 10 12:45:17 2018

Remove cross_thread_copier.h from document_encoding_data.h.

DocumentEncodingData is never passed across threads.

Bug: 242216, 689702
Change-Id: If5a78abdb39399c3283480310f5bbe1fab669700
Reviewed-on: https://chromium-review.googlesource.com/1131046
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573694}
[modify] https://crrev.com/2932b8bf7174dbd4ca26a7a8c162962824b15233/third_party/blink/renderer/core/dom/document_encoding_data.h

Sign in to add a comment