Convert XMLDocumentParser code to Use more reference

Convert XMLDocumentParser code to Use more reference

Document pointer can not be null to create XMLDocumentParser.
Move the parameter to use reference to make caller code safer.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173780

[maheshkk, 2014-05-08 19:34:34]

PTAL

[Inactive, 2014-05-08 19:45:41]

https://codereview.chromium.org/276733002/diff/1/Source/core/dom/DocumentPars...
File Source/core/dom/DocumentParser.cpp (left):

https://codereview.chromium.org/276733002/diff/1/Source/core/dom/DocumentPars...
Source/core/dom/DocumentParser.cpp:39: ASSERT(document);
Why did you remove this assertion? (you did not make the argument a reference).

https://codereview.chromium.org/276733002/diff/1/Source/core/html/parser/HTML...
File Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/276733002/diff/1/Source/core/html/parser/HTML...
Source/core/html/parser/HTMLDocumentParser.cpp:102:
HTMLDocumentParser::HTMLDocumentParser(HTMLDocument* document, bool
reportErrors)
I assume you are planning to update this to take a reference in a follow-up
patch? If so, maybe add a FIXME.

https://codereview.chromium.org/276733002/diff/1/Source/core/html/parser/HTML...
File Source/core/html/parser/HTMLViewSourceParser.cpp (right):

https://codereview.chromium.org/276733002/diff/1/Source/core/html/parser/HTML...
Source/core/html/parser/HTMLViewSourceParser.cpp:36:
HTMLViewSourceParser::HTMLViewSourceParser(HTMLViewSourceDocument* document,
const String& mimeType)
I assume you are planning to update this to take a reference in a follow-up
patch? If so, maybe add a FIXME.

https://codereview.chromium.org/276733002/diff/1/Source/core/inspector/DOMPat...
File Source/core/inspector/DOMPatchSupport.cpp (right):

https://codereview.chromium.org/276733002/diff/1/Source/core/inspector/DOMPat...
Source/core/inspector/DOMPatchSupport.cpp:100: parser =
XMLDocumentParser::create(*newDocument.get(), 0);
nit: drop the '.get()'

https://codereview.chromium.org/276733002/diff/1/Source/core/xml/parser/XMLDo...
File Source/core/xml/parser/XMLDocumentParser.h (right):

https://codereview.chromium.org/276733002/diff/1/Source/core/xml/parser/XMLDo...
Source/core/xml/parser/XMLDocumentParser.h:104: XMLDocumentParser(Document&,
FrameView* = 0);
nit: explicit

[maheshkk, 2014-05-08 20:02:31]

Thanks for review Chris. PTAL!

https://codereview.chromium.org/276733002/diff/1/Source/core/dom/DocumentPars...
File Source/core/dom/DocumentParser.cpp (left):

https://codereview.chromium.org/276733002/diff/1/Source/core/dom/DocumentPars...
Source/core/dom/DocumentParser.cpp:39: ASSERT(document);
On 2014/05/08 19:45:42, Chris Dumez wrote:
> Why did you remove this assertion? (you did not make the argument a
reference).

I forgot to undo this change. I originally moved DocumentParser to reference as
well, but will do that in different CL to keep this patch small enough to
review.

https://codereview.chromium.org/276733002/diff/1/Source/core/html/parser/HTML...
File Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/276733002/diff/1/Source/core/html/parser/HTML...
Source/core/html/parser/HTMLDocumentParser.cpp:102:
HTMLDocumentParser::HTMLDocumentParser(HTMLDocument* document, bool
reportErrors)
On 2014/05/08 19:45:42, Chris Dumez wrote:
> I assume you are planning to update this to take a reference in a follow-up
> patch? If so, maybe add a FIXME.

Yep. sure.

https://codereview.chromium.org/276733002/diff/1/Source/core/html/parser/HTML...
File Source/core/html/parser/HTMLViewSourceParser.cpp (right):

https://codereview.chromium.org/276733002/diff/1/Source/core/html/parser/HTML...
Source/core/html/parser/HTMLViewSourceParser.cpp:36:
HTMLViewSourceParser::HTMLViewSourceParser(HTMLViewSourceDocument* document,
const String& mimeType)
On 2014/05/08 19:45:42, Chris Dumez wrote:
> I assume you are planning to update this to take a reference in a follow-up
> patch? If so, maybe add a FIXME.

Done.

https://codereview.chromium.org/276733002/diff/1/Source/core/inspector/DOMPat...
File Source/core/inspector/DOMPatchSupport.cpp (right):

https://codereview.chromium.org/276733002/diff/1/Source/core/inspector/DOMPat...
Source/core/inspector/DOMPatchSupport.cpp:100: parser =
XMLDocumentParser::create(*newDocument.get(), 0);
On 2014/05/08 19:45:42, Chris Dumez wrote:
> nit: drop the '.get()'

Done.

https://codereview.chromium.org/276733002/diff/1/Source/core/xml/parser/XMLDo...
File Source/core/xml/parser/XMLDocumentParser.h (right):

https://codereview.chromium.org/276733002/diff/1/Source/core/xml/parser/XMLDo...
Source/core/xml/parser/XMLDocumentParser.h:104: XMLDocumentParser(Document&,
FrameView* = 0);
On 2014/05/08 19:45:42, Chris Dumez wrote:
> nit: explicit

Done.

[Inactive, 2014-05-08 20:20:48]

lgtm

[maheshkk, 2014-05-08 21:02:57]

The CQ bit was checked by mahesh.kk@samsung.com

[maheshkk, 2014-05-08 21:02:58]

The CQ bit was unchecked by mahesh.kk@samsung.com

[maheshkk, 2014-05-09 20:17:23]

The CQ bit was checked by mahesh.kk@samsung.com

[commit-bot: I haz the power, 2014-05-09 20:18:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mahesh.kk@samsung.com/276733002/20001

[commit-bot: I haz the power, 2014-05-09 20:32:48]

Change committed as 173780