もずはっく日記(2015年11月)

Bug-org 1218032 Ctrl-b while editing a mana page crashes the tab (crash in nsContentIterator::NextNode)
初回投稿日時: 2015年11月21日18時21分49秒
カテゴリ: IME Mozilla Core Mozilla45 バグ修正
SNS: Tweet (list)

Bug-org 1215798の修正によるregressionで、リッチテキストエディタでテキストを選択してスタイルを加えたり解除したりした時にクラッシュするというバグです。

nsContentIteratorが様々な状況下で様々なクラスのメソッドが期待通りの値を返してくるのを前提にして返されたポインタのnullptrチェックを怠っていたため、新たに発生するようになった状況下でこのクラッシュが発生するようになってしまっていました。

nsContentIteratorは様々な場所で使われているので安易な修正はさらなるregressionを呼び込みそうなので、今回の修正ではそのようなシチュエーションに陥らないように修正することにしました。そのシチュエーションとは、nsContentIteratorがDOMツリーから削除されているノードを参照していることです。この時、削除されたノードのルートは、nsINode::GetParent()でnullptrを返しますが、ここでクラッシュしていたのです。

何故この様なシチュエーションが発生してしまうのか検証してみたところ、ノードが削除された場合にIMEContentObserver::ContentRemoved()が呼び出され、テキストの変更通知をIME、もしくはContentCacheに伝える必要が出てきます。これはnsContentUtils::AddScriptRunner()に登録して発火を待ちますので、最速で全てのmutationが通知された直後に発火されます。おそらくこの時に選択範囲はまだ削除されたノードを参照している可能性があります。そうすると、選択範囲をキャッシュし直そうとする際に、nsContentIteratorが選択範囲のノードの祖先要素を取得しようとした際にクラッシュします。

おそらくこの問題により、e10sモードで多くのwarningがデバッグビルドを実行している最中には発火されていたと思われるので、この「危険な時間帯」にIMEやContentCacheにコンテンツの変更を通知しないようにすると色々と解決します。

今回の修正では、nsContentUtils::AddScriptRunner()を利用せずに、NS_DispatchToCurrentThread()を利用して通知を行うようにしました。ただ、単純にこのメソッドにだけ頼ると通知が遅くなりすぎて次のネイティブイベントを先にハンドリングしてしまう可能性があったり、多くのnsITextInputProcessorの自動テストを非同期で通知を受け取るように書き直す必要があるので、安全と思われるタイミングで強制的にフラッシュするようにしています。

まだ今回の修正では完全に安全な状況以外でも呼び出しが発生してると思われるwarningが散見されますが、以前よりはかなり状況限定になっている様です。