Add null check to fix crash in blink::ComputedStyle::display

third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp

 /*
  * Copyright (C) 2005, 2006, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2009, 2010, 2011 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 776 matching lines @@
             || isHTMLMetaElement(*node)
             || isHTMLTitleElement(*node)) {
             next = NodeTraversal::nextSkippingChildren(*node);
             fragment.removeNode(node);
         } else {
             next = NodeTraversal::next(*node);
         }
     }
 }
 
+static bool isListItemStyle(const HTMLElement& element)

[yosin_UTC9, 2016/07/12 01:30:36]

Can make |isListItem()| in "EditingUtilitis.h" to use computed style and use it
here?

[joone, 2016/07/12 23:08:54]

Done.

+{
+    if (element.computedStyle() && element.computedStyle()->display() == LIST_ITEM)

[yosin_UTC9, 2016/07/12 01:30:36]

nit: We should avoid to use |return true| with |return false|

const ComputedStyle* style = element.computedStyle();
return style && style->display() == LIST_ITEM;

[joone, 2016/07/12 23:08:54]

Done.

+        return true;
+    return false;
+}
+
+static bool isTableCellStyle(const HTMLElement& element)

[yosin_UTC9, 2016/07/12 01:30:36]

Can make |isTableCell()| in "EditingUtilitis.h" to use computed style and use it
here?

[joone, 2016/07/12 23:08:54]

Done.

+{
+    if (element.computedStyle() && element.computedStyle()->display() == TABLE_CELL)

[yosin_UTC9, 2016/07/12 01:30:36]

nit: We should avoid to use |return true| with |return false|

const ComputedStyle* style = element.computedStyle();
return style && style->display() == LIST_ITEM;

[joone, 2016/07/12 23:08:54]

Done.

+        return true;
+    return false;
+}
+
 static bool followBlockElementStyle(const Node* node)
 {
     if (!node->isHTMLElement())
         return false;
 

[yosin_UTC9, 2016/07/12 01:30:36]

I think computedStyle() should not be return nullptr, but current implementation
returns nullptr.
I guess this situation is changed by "Splitting up Style Resolution and Layout
Tree Construction"
https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/style$20t...


It seems null-pointer reference is occurred at L834 of
handleStyleSpansBeforeInsertion() instead of here:

834 if (node->computedStyle()->display() == INLINE) {
        node = enclosingBlock(insertionPos.anchorNode());
        if (!node)
            return false;
    }

    if (followBlockElementStyle(node)) {
        fragment.removeNodePreservingChildren(wrappingStyleSpan);
        return true;
    }

[joone, 2016/07/12 23:08:54]

We can use isInline() function defined in EditingUtilities.cpp

     const HTMLElement& element = toHTMLElement(*node);
-    return element.computedStyle()->display() == LIST_ITEM
-        || element.computedStyle()->display() == TABLE_CELL
+    return isListItemStyle(element)
+        || isTableCellStyle(element)
         || element.hasTagName(preTag)
         || element.hasTagName(h1Tag)
         || element.hasTagName(h2Tag)
         || element.hasTagName(h3Tag)
         || element.hasTagName(h4Tag)
         || element.hasTagName(h5Tag)
         || element.hasTagName(h6Tag);
 }
 
 // Remove style spans before insertion if they are unnecessary.  It's faster because we'll
@@ skipping 941 matching lines @@
     visitor->trace(m_startOfInsertedContent);
     visitor->trace(m_endOfInsertedContent);
     visitor->trace(m_insertionStyle);
     visitor->trace(m_documentFragment);
     visitor->trace(m_startOfInsertedRange);
     visitor->trace(m_endOfInsertedRange);
     CompositeEditCommand::trace(visitor);
 }
 
 } // namespace blink