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

Bug-org 1166436 crash in mozilla::dom::TabParent::HandleQueryContentEvent(mozilla::WidgetQueryContentEvent&)
初回投稿日時: 2015年06月06日15時33分38秒
最終更新日時: 2015年06月06日15時34分05秒
カテゴリ: e10s Mozilla Core Mozilla41 バグ修正
SNS: Tweet (list)

Windows x64版でe10sが有効かつ、IMMモード(現在、Nightlyでもe10sモードの場合はIMMモードが標準)の場合、TabParentがNS_QUERY_TEXT_RECTをハンドリング中にクラッシュする、というバグです。ちなみに、crash reportを見てみると、Macでも希に発生している模様です。

散々デバッグした結果、その原因は、TabParentがキャッシュしている未確定文字列の各文字の矩形にアクセスする際のチェックにありました。もともと、おかしな値であるUINT32_MAXがオフセットとして指定されている場合限定のバグなのですが、取得する文字数がゼロではない場合、そのオフセット(UINT32_MAX)と長さを足すことで、uint32_tからオーバーフローし、小さな値になります。その結果、オフセットは、キャッシュしてる文字の矩形の最初のオフセット以上かつ、キャッシュしてる最後の文字の矩形のオフセット以下という判定にパスしてしまいます。C++に詳しくないので、なぜこれがx64ビルドでのみ発生しているのかその原因は分かりませんが、ありえないオフセットが来た場合のチェックが甘いことがクラッシュの原因でした。

TabParentは元々、Android版のFirefoxを開発時に段階的に実装され、e10s対応でさらにコンテンツのキャッシュが増え、コードがかなり見づらい状態に陥っていましたので、上記の原因はコードを読んでいるだけでは発見できませんでした。そこで、検証をより簡単にするために、リファクタリングを行うことにしました。

現在のTabParentは、PuppetWidgetがIMEへの通知を受け取る度に、様々な情報を更新していくスタイルです。しかし、非同期でこれが行われているため、TabParentからすると、一部の情報のみが更新されているタイミングで、IMEからコンテンツの問い合わせが来るかもしれません。その場合、TSFではよくあるのですが、想定外の問い合わせ結果を返してしまうとTSFやTIP内部でクラッシュしてしまうことがあります。これを解決するには、PuppetWidget側で必要な情報を揃えてからTabParent側で一括でキャッシュを更新してしまう必要があります。

これらを踏まえて新しいデザインを考えた結果、PuppetWidgetとTabParent双方で同じクラスをデータの保存のために利用し、IMEに対して何らかの変更通知を出す際に、PuppetWidget側で必要な情報を揃えてから、通知に添えて新しいコンテンツの情報を送信するようにしました。

この新しいクラスは、mozilla::ContentCacheで、メンバ変数は全てprivateで、直接それらにアクセスすることも今はできません。その代わりにこのクラスがWidgetQueryContentEventを直接ハンドリングしますし、PuppetWidgetの代わりにコンテンツの情報を必要に応じて自動的に取得するようにしています(例えばテキストが変更された場合は、選択範囲や、テキストの矩形も変化している可能性が高いので自動的に取得します)。

また、デバッグを容易にするため、ログも取れるようになっています。NSPR_LOG_MODULES=ContentCacheWidgets:5で動作ログと、キャッシュの情報が取得できますので、デバッグの際にはご利用ください。

選択範囲の開始位置や終了位置の文字の矩形もキャッシュするようにしたり、キャレットの問い合わせ位置に実際にキャレットが無かった場合は、文字の矩形のキャッシュが無いか調査し、もしあれば、そこから推測してキャレットの矩形を返すようにもなっていますので、いくつかのケースで動作が改善しているかもしれません。