Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(405)

Issue 1910153002: Reland of DocumentLoader should commit the load before creating a DocumentWriter (Closed)

Created:
4 years, 8 months ago by meacer
Modified:
4 years, 8 months ago
CC:
chromium-reviews, blink-reviews, loading-reviews_chromium.org, tyoshino+watch_chromium.org, Nate Chapin, gavinp+loader_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland of DocumentLoader should commit the load before creating a DocumentWriter The current order of events in DocumentLoader causes an HTML parser to be created via DocumentWriter::create before the load commits. This causes the HTML parser to read a stale value for scriptEnabled setting in HTMLParserOptions if the setting changes between page loads. This in turn breaks the rendering of noscript tags. This CL moves receivedFirstData call before DocumentWriter::create and FrameLoader::didBeginDocument so that the load is committed before the HTML parser is created and the correct scriptEnabled value is used by the parser. It also moves creation of Content Security Policy and setting saved form data to a new callback didInstallNewDocument. NOTE: This also changes the order of callbacks in WebFrameClient. The OLD order when loading a document was: 1. didStartProvisionalLoad 2. didCreateNewDocument 3. didCommitProvisionalLoad 4. didCreateDocumentElement The NEW order is: 1. didStartProvisionalLoad 2. didCommitProvisionalLoad 3. didCreateNewDocument 4. didCreateDocumentElement BUG=232410 TBR=jochen,dcheng Committed: https://crrev.com/b4d6bf98e54db861b8d9adc7642c47b1677b8f40 Cr-Commit-Position: refs/heads/master@{#388944}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+173 lines, -51 lines) Patch
M chrome/renderer/content_settings_observer_browsertest.cc View 7 chunks +142 lines, -33 lines 0 comments Download
M third_party/WebKit/Source/core/loader/DocumentLoader.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/DocumentLoader.cpp View 3 chunks +14 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/core/loader/FrameLoader.h View 1 chunk +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/FrameLoader.cpp View 2 chunks +13 lines, -6 lines 0 comments Download

Messages

Total messages: 9 (4 generated)
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1910153002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1910153002/1
4 years, 8 months ago (2016-04-21 19:57:33 UTC) #2
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/209277)
4 years, 8 months ago (2016-04-21 20:34:46 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1910153002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1910153002/1
4 years, 8 months ago (2016-04-21 20:57:37 UTC) #6
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 8 months ago (2016-04-21 23:14:20 UTC) #7
commit-bot: I haz the power
4 years, 8 months ago (2016-04-22 19:41:19 UTC) #9
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/b4d6bf98e54db861b8d9adc7642c47b1677b8f40
Cr-Commit-Position: refs/heads/master@{#388944}

Powered by Google App Engine
This is Rietveld 408576698