Add "magic values" to HTMLFrameOwnerElement and Frame to help diagnose crash

I've narrowed down the linked crash to a bad m_owner pointer in Frame. I've
added two temporary magic values to help diagnose where this is coming from:

In Frame I added a magic value beside the m_owner element to guard against
heap corruption.

In HTMLFrameOwnerElement I added a magic value that should help us catch when
m_owner is not pointing to an HTMLFrameOwnerElement sooner.

In both cases, we should also be able to see if either m_owner of Frame has
already been destructed.

Also turned the ASSERT in ~HTMLFrameOwnerElement into a RELEASE_ASSERT in
case we're destroying it without disconnecting it from the Frame.

BUG=519752

Review URL: https://codereview.chromium.org/1369003003

Cr-Commit-Position: refs/heads/master@{#351150}

third_party/WebKit/Source/core/frame/Frame.cpp

@@ -68,6 +68,7 @@ DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, frameCounter, ("Frame"));
 
 Frame::~Frame()
 {
+    m_ownerMagicValue = 0xdeadcafe;
     InstanceCounters::decrementCounter(InstanceCounters::FrameCounter);
     ASSERT(!m_owner);
 #ifndef NDEBUG
@@ -292,11 +293,14 @@ Settings* Frame::settings() const
 Frame::Frame(FrameClient* client, FrameHost* host, FrameOwner* owner)
     : m_treeNode(this)
     , m_host(host)
+    , m_ownerMagicValue(0xcafecafe)
     , m_owner(owner)
     , m_client(client)
     , m_frameID(generateFrameID())
     , m_isLoading(false)
 {
+    verifyOwnerPointerAndCrashIfNecessary();
+
     InstanceCounters::incrementCounter(InstanceCounters::FrameCounter);
 
     ASSERT(page());
@@ -313,4 +317,16 @@ Frame::Frame(FrameClient* client, FrameHost* host, FrameOwner* owner)
     }
 }
 
+void Frame::verifyOwnerPointerAndCrashIfNecessary()
+{
+    // If this fails the m_owner pointer was likely also overwritten.
+    RELEASE_ASSERT(m_ownerMagicValue == 0xcafecafe);
+
+    // If this fails the m_owner pointer isn't pointing to an HTMLFrameOwnerElement or
+    // it's been destructed.
+    if (HTMLFrameOwnerElement* owner = deprecatedLocalOwner()) {
+        RELEASE_ASSERT(owner->m_magicValue == 0xdecaf);
+    }
+}
+
 } // namespace blink

third_party/WebKit/Source/core/frame/Frame.h

@@ -95,7 +95,12 @@ public:
     bool isLocalRoot() const;
 
     FrameOwner* owner() const;
-    void setOwner(FrameOwner* owner) { m_owner = owner; }
+    void setOwner(FrameOwner* owner)
+    {
+        m_owner = owner;
+        verifyOwnerPointerAndCrashIfNecessary();
+    }
+
     HTMLFrameOwnerElement* deprecatedLocalOwner() const;
 
     FrameTree& tree() const;
@@ -129,6 +134,8 @@ public:
     void setIsLoading(bool isLoading) { m_isLoading = isLoading; }
     bool isLoading() const { return m_isLoading; }
 
+    void verifyOwnerPointerAndCrashIfNecessary();
+
 protected:
     Frame(FrameClient*, FrameHost*, FrameOwner*);
 
@@ -137,6 +144,10 @@ protected:
     mutable FrameTree m_treeNode;
 
     RawPtrWillBeMember<FrameHost> m_host;
+
+    // TODO(bokan): Temporary, used for debugging crbug.com/519752. This will help
+    // check if we've overwritten the m_owner pointer.
+    volatile unsigned m_ownerMagicValue;
     RawPtrWillBeMember<FrameOwner> m_owner;
 
 private:

third_party/WebKit/Source/core/frame/LocalFrame.cpp

@@ -148,6 +148,7 @@ void LocalFrame::createView(const IntSize& viewportSize, const Color& background
     ScrollbarMode horizontalScrollbarMode, bool horizontalLock,
     ScrollbarMode verticalScrollbarMode, bool verticalLock)
 {
+    verifyOwnerPointerAndCrashIfNecessary();
     ASSERT(this);
     ASSERT(page());
 

third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp

@@ -106,6 +106,7 @@ static void moveWidgetToParentSoon(Widget* child, FrameView* parent)
 
 HTMLFrameOwnerElement::HTMLFrameOwnerElement(const QualifiedName& tagName, Document& document)
     : HTMLElement(tagName, document)
+    , m_magicValue(0xdecaf)
     , m_contentFrame(nullptr)
     , m_widget(nullptr)
     , m_sandboxFlags(SandboxNone)
@@ -167,9 +168,11 @@ void HTMLFrameOwnerElement::disconnectContentFrame()
 
 HTMLFrameOwnerElement::~HTMLFrameOwnerElement()
 {
+    m_magicValue = 0xdeadcaf;
+
     // An owner must by now have been informed of detachment
     // when the frame was closed.
-    ASSERT(!m_contentFrame);
+    RELEASE_ASSERT(!m_contentFrame);
 }
 
 Document* HTMLFrameOwnerElement::contentDocument() const

third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.h

@@ -85,6 +85,11 @@ public:
 
     DECLARE_VIRTUAL_TRACE();
 
+    // TODO(bokan): Temporary, used for debugging crbug.com/519752. This will
+    // help us tell that a LocalFrame's m_owner pointer is actually pointing to
+    // an HTMLFrameOwnerElement.
+    volatile unsigned m_magicValue;
+
 protected:
     HTMLFrameOwnerElement(const QualifiedName& tagName, Document&);
     void setSandboxFlags(SandboxFlags);