Remove WebURLResponse::initialize() [revised]

third_party/WebKit/Source/platform/exported/WebURLResponse.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 21 matching lines @@
 
 #include "platform/exported/WebURLResponsePrivate.h"
 #include "platform/network/ResourceLoadTiming.h"
 #include "platform/network/ResourceResponse.h"
 #include "public/platform/WebHTTPHeaderVisitor.h"
 #include "public/platform/WebHTTPLoadInfo.h"
 #include "public/platform/WebString.h"
 #include "public/platform/WebURL.h"
 #include "public/platform/WebURLLoadTiming.h"
 #include "wtf/Allocator.h"
+#include "wtf/Assertions.h"
 #include "wtf/PtrUtil.h"
 #include "wtf/RefPtr.h"
 #include <memory>
 
 namespace blink {
 
 namespace {
 
 class ExtraDataContainer : public ResourceResponse::ExtraData {
 public:
@@ skipping 17 matching lines @@
 // The standard implementation of WebURLResponsePrivate, which maintains
 // ownership of a ResourceResponse instance.
 class WebURLResponsePrivateImpl final : public WebURLResponsePrivate {
     USING_FAST_MALLOC(WebURLResponsePrivateImpl);
 public:
     WebURLResponsePrivateImpl()
     {
         m_resourceResponse = &m_resourceResponseAllocation;
     }
 
-    WebURLResponsePrivateImpl(const WebURLResponsePrivate* p)
-        : m_resourceResponseAllocation(*p->m_resourceResponse)
+    WebURLResponsePrivateImpl(const WebURLResponsePrivate& p)
+        : m_resourceResponseAllocation(*p.m_resourceResponse)
     {
         m_resourceResponse = &m_resourceResponseAllocation;
     }
 
-    virtual void dispose() { delete this; }
+    ~WebURLResponsePrivateImpl() override {}
 
 private:
-    virtual ~WebURLResponsePrivateImpl() { }
-
     ResourceResponse m_resourceResponseAllocation;
 };
 
-void WebURLResponse::initialize()
+WebURLResponse::~WebURLResponse()
 {
-    assign(new WebURLResponsePrivateImpl());
 }
 
-void WebURLResponse::reset()
+WebURLResponse::WebURLResponse()
+    : m_owningPrivate(new WebURLResponsePrivateImpl())
+    , m_private(m_owningPrivate.get())
 {
-    assign(0);
 }
 
-void WebURLResponse::assign(const WebURLResponse& r)
+WebURLResponse::WebURLResponse(const WebURLResponse& r)
+    : m_owningPrivate(new WebURLResponsePrivateImpl(*r.m_private))
+    , m_private(m_owningPrivate.get())
 {
+}
+
+WebURLResponse::WebURLResponse(const WebURL& url)
+    : WebURLResponse()
+{
+    setURL(url);
+}
+
+WebURLResponse& WebURLResponse::operator=(const WebURLResponse& r)
+{
+    // Copying subclasses that have different m_private ownership semantics
+    // via this operator is just not supported.
+    DCHECK(m_owningPrivate);
+    DCHECK(m_private->m_resourceResponse);
     if (&r != this)
-        assign(r.m_private ? new WebURLResponsePrivateImpl(r.m_private) : 0);
+        *m_private->m_resourceResponse = *r.m_private->m_resourceResponse;
+    return *this;
 }
 
 bool WebURLResponse::isNull() const
 {
-    return !m_private || m_private->m_resourceResponse->isNull();
+    return !m_private->m_resourceResponse || m_private->m_resourceResponse->isNull();

[dcheng, 2016/07/07 05:02:50]

It looks like this can only happen when using the default constructor of
WrappedResourceRequest. If codesearch is accurate, the only case where this
happens in WebDataSourceImpl. But WebDataSourceImpl::response() always binds
DocumentLoader::response() before returning.

Does it makes sense to:
- Always bind the response in WebDataSourceImpl ctor
- Remove the default ctor for WrappedResourceResponse
- Remove the !m_private->m_resourceResponse check, since I /think/ it will be
non-null in all other cases.

[kinuko, 2016/07/07 09:23:04]

Looks like the change works, and I like simplifying code.  Done.

 }
 
 WebURL WebURLResponse::url() const
 {
     return m_private->m_resourceResponse->url();
 }
 
 void WebURLResponse::setURL(const WebURL& url)
 {
     m_private->m_resourceResponse->setURL(url);
@@ skipping 213 matching lines @@
         webSecurityDetails.mac,
         webSecurityDetails.certId,
         webSecurityDetails.numUnknownScts,
         webSecurityDetails.numInvalidScts,
         webSecurityDetails.numValidScts,
         sctList);
 }
 
 ResourceResponse& WebURLResponse::toMutableResourceResponse()
 {
-    ASSERT(m_private);
-    ASSERT(m_private->m_resourceResponse);
-
+    DCHECK(m_private->m_resourceResponse);
     return *m_private->m_resourceResponse;
 }
 
 const ResourceResponse& WebURLResponse::toResourceResponse() const
 {
-    ASSERT(m_private);
-    ASSERT(m_private->m_resourceResponse);
-
+    DCHECK(m_private->m_resourceResponse);
     return *m_private->m_resourceResponse;
 }
 
 bool WebURLResponse::wasCached() const
 {
     return m_private->m_resourceResponse->wasCached();
 }
 
 void WebURLResponse::setWasCached(bool value)
 {
@@ skipping 138 matching lines @@
     if (!data)
         return 0;
     return static_cast<ExtraDataContainer*>(data.get())->getExtraData();
 }
 
 void WebURLResponse::setExtraData(WebURLResponse::ExtraData* extraData)
 {
     m_private->m_resourceResponse->setExtraData(ExtraDataContainer::create(extraData));
 }
 
-void WebURLResponse::assign(WebURLResponsePrivate* p)
+WebURLResponse::WebURLResponse(WebURLResponsePrivate* p)
+    : m_private(p)
 {
-    // Subclasses may call this directly so a self-assignment check is needed
-    // here as well as in the public assign method.
-    if (m_private == p)
-        return;
-    if (m_private)
-        m_private->dispose();
-    m_private = p;
+    DCHECK(p);
 }
 
 } // namespace blink