Revert "Filter out empty ID in attribute changed callback"

This reverts commit 103ac117.

It appears that we were overly optimistic. There can be intermediate
states where Element::GetIdAttribute() is called from
Element::UpdateId(), which violates the DCHECK() put into place in the
reverted commit.

Original change's description:
> Filter out empty ID in attribute changed callback
>
> Per spec [1], empty ID attributes must be treated identically as
> nonexistent. This CL implements these steps faithfully, and results in
> DocumentFragment::getElementById() always returning nullptr when the
> empty string is given, thus fixing bug 1068580.
>
> Additionally, previously Document::getElementById() explicitly returned
> nullptr when the empty string or the null string is given. The empty
> string handling is no longer necessary after this CL, and thus removed.
> (The null string handling is still necessary for internal C++ code that
> relies on the short-circuiting behavior).
>
> [1]: https://dom.spec.whatwg.org/#concept-id
>
> Change-Id: Ia65024efef9e5ef54d9db233590a17b372282ed1
> Fixed: 1068580
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2140054
> Reviewed-by: Kent Tamura <tkent@chromium.org>
> Commit-Queue: Timothy Gu <timothygu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#757310}

Fixed: 1069728
Change-Id: I70b2fd8d14456ec0b063cb18e5982384bd1285bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146924Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Timothy Gu <timothygu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758436}

third_party/blink/renderer/core/dom/element.cc

@@ -2451,11 +2451,6 @@ void Element::AttributeChanged(const AttributeModificationParams& params) {
     AtomicString old_id = GetElementData()->IdForStyleResolution();
     AtomicString new_id = MakeIdForStyleResolution(
         params.new_value, GetDocument().InQuirksMode());
-    // Treat empty IDs the same way as nonexistent ID, per
-    // https://dom.spec.whatwg.org/#concept-id.
-    if (new_id.IsEmpty()) {
-      new_id = g_null_atom;
-    }
     if (new_id != old_id) {
       GetElementData()->SetIdForStyleResolution(new_id);
       GetDocument().GetStyleEngine().IdChangedForElement(old_id, new_id, *this);

third_party/blink/renderer/core/dom/element.h

@@ -1299,17 +1299,7 @@ inline const AtomicString& Element::IdForStyleResolution() const {
 }
 
 inline const AtomicString& Element::GetIdAttribute() const {
-  // Note: HasID() can return false even if the id attribute exists. This
-  // happens when the attribute value is empty, which per spec is equivalent to
-  // not having an id attribute at all:
-  // https://dom.spec.whatwg.org/#concept-id. On the other hand, if HasID()
-  // returns true then there must be a non-empty id attribute.
-  if (HasID()) {
-    const AtomicString& attr_value = FastGetAttribute(html_names::kIdAttr);
-    DCHECK(!attr_value.IsEmpty());
-    return attr_value;
-  }
-  return g_null_atom;
+  return HasID() ? FastGetAttribute(html_names::kIdAttr) : g_null_atom;
 }
 
 inline const AtomicString& Element::GetNameAttribute() const {

third_party/blink/renderer/core/dom/tree_scope.cc

@@ -110,9 +110,7 @@ void TreeScope::ClearScopedStyleResolver() {
 }
 
 Element* TreeScope::getElementById(const AtomicString& element_id) const {
-  // |element_id| can never be null for calls that originated from JavaScript,
-  // but sometimes other C++ code can call this function with g_null_atom.
-  if (element_id.IsNull())
+  if (element_id.IsEmpty())
     return nullptr;
   if (!elements_by_id_)
     return nullptr;

third_party/blink/web_tests/external/wpt/dom/nodes/DocumentFragment-getElementById-expected.txt

+This is a testharness.js-based test.
+PASS The method must exist
+PASS It must return null when there are no matches
+PASS It must return the first element when there are matches
+FAIL Empty string ID values assert_equals: Even if there is an element with an empty-string ID attribute, it must not be returned expected null but got Element node <div id=""></div>
+PASS It must return the first element when there are matches, using a template
+Harness: the test ran to completion.
+