opendev/gerrit
Code Issues Proposed changes

Nest gr-account-list's chips dom-repeat in a div for race-condition

Browse Source 
As the PolyGerrit change view makes API requests to load a change, in
rare cases, the API server will take so long to respond with the server
config (at /config/server/info) that it will not arrive until after the
change detail (at /changes/##/detail) has resolved. This slowness falls
within normal fluctuations of responsiveness according to server load.

However, as PolyGerrit renders UI in response to APIs coming in, this
rare, unusual order of request resolution times causes a chain of
Polymer computation effects that results in a DOM manipulation
exception. This exception prevents complete rendering of the reply
dialog; namely it fails to render the CC list element.

When a user clicks on the [Submit] button for a change with a broken
reply dialog, the handler checks whether the CC list should have
appeared in the UI (by checking whether NoteDB is enabled in the server
config). If it sees that it should, it attempts to extract the added CC
entries from the list which has not rendered -- resulting in another
exception, which, this time, prevents any reply from being submitted.

The chain of computation effects that result in the initial rendering
error is indirect. When initializing a gr-autocomplete-dropdown, the
suggestion list is initialized to an empty set. Any change in the
suggestion list triggers a recomputation of the suggestion elements (of
which there are none). Querying for the suggestion elements is preceded
by a Polymer.dom.flush() call in order to ensure that they have been
rendered.

When this process flushes the DOM, one of the operations that is queued
will be to render the chips in a gr-account-list's dom-repeat. When
rendering, each instantiated chip is prepended to that element's tree
using a native .insertBefore call with the new chip being passed as the
first parameter and the dom-repeat being passed as the second parameter.
However, for some reason, the dom-repeat's parentElement registers as
null at this point, resulting in the DOM exception.

Nesting the dom-repeat in a div results in the correct parentElement
reference in this case.

Bug: Issue 6419
Change-Id: Ic5a07f0e168285d9baf35126609ecce8df2941a7
This commit is contained in:
Wyatt Allen
2017-07-21 10:31:56 -07:00
parent b267eb5819
commit 3f8bdcc65d
1 changed files with 16 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html
View File

@@ -38,16 +38,22 @@ limitations under the License.
font-style: italic;
}
</style>
<template id="chips" is="dom-repeat" items="[[accounts]]" as="account">
<gr-account-chip
account="[[account]]"
class$="[[_computeChipClass(account)]]"
data-account-id$="[[account._account_id]]"
removable="[[_computeRemovable(account)]]"
on-keydown="_handleChipKeydown"
tabindex="-1">
</gr-account-chip>
</template>
<!--
NOTE(Issue 6419): Nest the inner dom-repeat template in a div rather than
as a direct child of the dom-module's template.
-->
<div>
<template id="chips" is="dom-repeat" items="[[accounts]]" as="account">
<gr-account-chip
account="[[account]]"
class$="[[_computeChipClass(account)]]"
data-account-id$="[[account._account_id]]"
removable="[[_computeRemovable(account)]]"
on-keydown="_handleChipKeydown"
tabindex="-1">
</gr-account-chip>
</template>
</div>
<gr-account-entry
borderless
hidden$="[[_computeEntryHidden(maxCount, accounts.*, readonly)]]"