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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 277633002: Convert DocumentParser code to Use more reference (Closed)

Created:
6 years, 7 months ago by maheshkk
Modified:
6 years, 7 months ago
Reviewers:
Inactive
CC:
blink-reviews, eustas+blink_chromium.org, caseq+blink_chromium.org, blink-reviews-html_chromium.org, loislo+blink_chromium.org, pfeldman+blink_chromium.org, malch+blink_chromium.org, sof, eae+blinkwatch, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, blink-reviews-dom_chromium.org, dglazkov+blink, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Convert DocumentParser code to Use more reference Document pointer can not be null to create DocumentParser. Also HTMLViewSourceParser parameter can not be null. Move the parameter to use reference to make caller code safer. This is follow up patch from previous CL# 276733002. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174654

Patch Set 1 : Update rebased patch #

Total comments: 2

Patch Set 2 : Make TextDocument use reference as well #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -23 lines) Patch
M Source/core/dom/Document.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/HTMLViewSourceDocument.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/TextDocument.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/parser/HTMLDocumentParser.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/html/parser/HTMLDocumentParser.cpp View 1 chunk +7 lines, -8 lines 0 comments Download
M Source/core/html/parser/HTMLViewSourceParser.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/html/parser/HTMLViewSourceParser.cpp View 1 chunk +3 lines, -4 lines 0 comments Download
M Source/core/html/parser/TextDocumentParser.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/html/parser/TextDocumentParser.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/inspector/DOMPatchSupport.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
maheshkk
PTAL
6 years, 7 months ago (2014-05-22 19:51:54 UTC) #1
Inactive
https://codereview.chromium.org/277633002/diff/20001/Source/core/html/parser/TextDocumentParser.cpp File Source/core/html/parser/TextDocumentParser.cpp (right): https://codereview.chromium.org/277633002/diff/20001/Source/core/html/parser/TextDocumentParser.cpp#newcode36 Source/core/html/parser/TextDocumentParser.cpp:36: : HTMLDocumentParser(*document, false) This looks unsafe unless TextDocumentParser() constructor ...
6 years, 7 months ago (2014-05-22 19:58:43 UTC) #2
maheshkk
https://codereview.chromium.org/277633002/diff/20001/Source/core/html/parser/TextDocumentParser.cpp File Source/core/html/parser/TextDocumentParser.cpp (right): https://codereview.chromium.org/277633002/diff/20001/Source/core/html/parser/TextDocumentParser.cpp#newcode36 Source/core/html/parser/TextDocumentParser.cpp:36: : HTMLDocumentParser(*document, false) On 2014/05/22 19:58:44, Chris Dumez wrote: ...
6 years, 7 months ago (2014-05-22 20:15:53 UTC) #3
Inactive
lgtm
6 years, 7 months ago (2014-05-23 03:52:28 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mahesh.kk@samsung.com/277633002/40001
6 years, 7 months ago (2014-05-23 03:53:32 UTC) #5
commit-bot: I haz the power
6 years, 7 months ago (2014-05-23 05:11:49 UTC) #6
Message was sent while issue was closed. 
Change committed as 174654

Powered by Google App Engine
This is Rietveld 408576698