Have StyleElement API take Document as a reference

Have StyleElement API take Document as a reference

Have StyleElement API take Document as a reference instead of a pointer
since the document cannot be NULL. This also avoid converting the
document reference into a pointer in call sites.

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

[abarth-chromium, 2013-08-31 18:54:23]

If we accidentally assigned to |document|, would that change the value of
m_document somewhere?  If so, that seems kind of scary.  I guess we'll need to
be careful.

LGTM

[do-not-use, 2013-08-31 20:11:51]

On 2013/08/31 18:54:23, abarth wrote:
> If we accidentally assigned to |document|, would that change the value of
> m_document somewhere?  If so, that seems kind of scary.  I guess we'll need to
> be careful.

I have tried the following and it did not build:
--- a/Source/core/dom/StyleElement.cpp
+++ b/Source/core/dom/StyleElement.cpp
@@ -70,6 +70,9 @@ void StyleElement::removedFromDocument(Document& document,
Element* element, Con
     ASSERT(element);
     document.styleSheetCollections()->removeStyleSheetCandidateNode(element,
scopingNode);
 
+    RefPtr<Document> doc = Document::create();
+    document = *doc.get();

../../third_party/WebKit/Source/core/platform/TreeShared.h: In member function
‘WebCore::Node& WebCore::Node::operator=(const WebCore::Node&)’:
../../third_party/WebKit/Source/core/platform/TreeShared.h:36:57: error:
‘WebCore::TreeShared<NodeType>& WebCore::TreeShared<NodeType>::operator=(const
WebCore::TreeShared<NodeType>&) [with NodeType = WebCore::Node;
WebCore::TreeShared<NodeType> = WebCore::TreeShared<WebCore::Node>]’ is private

A Document is not copyable.

[commit-bot: I haz the power, 2013-08-31 20:59:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/23847002/1

[commit-bot: I haz the power, 2013-08-31 22:33:35]

Change committed as 157042

[abarth-chromium, 2013-09-01 14:51:42]

On 2013/08/31 20:11:51, Christophe Dumez wrote:
> A Document is not copyable.

A good point.  We should be careful to make types non-copyable when we apply
this technique.

[do-not-use, 2013-09-03 09:54:14]

On 2013/09/01 14:51:42, abarth wrote:
> On 2013/08/31 20:11:51, Christophe Dumez wrote:
> > A Document is not copyable.
> 
> A good point.  We should be careful to make types non-copyable when we apply
> this technique.

Well, actually, if the type was copyable you could already do that with a
pointer:
*document = *doc; // Would potentially change m_document somewhere via the
document pointer.

Therefore, I don't think using references over pointers is really ever riskier.

[abarth-chromium, 2013-09-03 20:52:17]

On 2013/09/03 09:54:14, Christophe Dumez wrote:
> Well, actually, if the type was copyable you could already do that with a
> pointer:
> *document = *doc; // Would potentially change m_document somewhere via the
> document pointer.

Yes, but that's easier to see as a problem in a code review than

document = doc;

which looks just like a normal variable assignment.

In any case, this is all find for Document because it's not copiable.  We should
just be careful when mixing non-const references with copiable types.