Crash fix: Avoid using stale HTMLToken after tree construction

third_party/WebKit/Source/core/html/parser/HTMLTreeBuilder.cpp

 /*
  * Copyright (C) 2010 Google, Inc. All Rights Reserved.
  * Copyright (C) 2011, 2014 Apple 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 774 matching lines @@
         || token->name() == imgTag
         || token->name() == keygenTag
         || token->name() == wbrTag) {
         m_tree.reconstructTheActiveFormattingElements();
         m_tree.insertSelfClosingHTMLElement(token);
         m_framesetOk = false;
         return;
     }
     if (token->name() == inputTag) {
         Attribute* typeAttribute = token->getAttributeItem(typeAttr);
+        bool disableFrameset = !typeAttribute || !equalIgnoringCase(typeAttribute->value(), "hidden");
+
         m_tree.reconstructTheActiveFormattingElements();
         m_tree.insertSelfClosingHTMLElement(token);
-        if (!typeAttribute || !equalIgnoringCase(typeAttribute->value(), "hidden"))
-            m_framesetOk = false;
+        m_framesetOk = disableFrameset;

[Yoav Weiss, 2015/09/29 07:12:42]

Can you detail what you suspect is happening in the minimized test case? What
makes you think this change will give us a better picture of what's going on?

A few more questions about this specific change:
* We could just get rid of disableFrameset and assign the expression directly to
m_framesetOk, right?
* IIUC, that change will cause a behavior change since we'd be now changing the
state of m_framesetOk
 - When disableFrameset is true, m_framesetOk will be true when previously it
was false
 - When disableFrameset is false, m_framesetOk will be false when previously it
maintained its previous state.
* Is that behavior change intentional? If so, can we test it? (I'm guessing it
is not since I don't see framesets on the test case in
https://code.google.com/p/chromium/issues/detail?id=468406 )

[kouhei (in TOK), 2015/09/29 07:32:18]

Thanks for the catch. This wasn't intentional. Let me try to write a test for
this again... and first I need to understand why this logic is here in the first
place.

         return;
     }
     if ((RuntimeEnabledFeatures::contextMenuEnabled() && token->name() == menuitemTag)
         || token->name() == paramTag
         || token->name() == sourceTag
         || token->name() == trackTag) {
         m_tree.insertSelfClosingHTMLElement(token);
         return;
     }
     if (token->name() == hrTag) {
@@ skipping 1992 matching lines @@
     ASSERT(m_isAttached);
     // Warning, this may detach the parser. Do not do anything else after this.
     m_tree.finishedParsing();
 }
 
 void HTMLTreeBuilder::parseError(AtomicHTMLToken*)
 {
 }
 
 } // namespace blink