もずはっく日記(2013年10月)

Bug-org 920425 WidgetEvent should have As*Event() methods for capsuling event struct check and static cast
初回投稿日時: 2013年10月31日14時59分44秒
最終更新日時: 2013年10月31日15時01分10秒
カテゴリ: Mozilla Core Mozilla27 バグ修正
SNS: Tweet (list)

WidgetEvent::As*Event()を実装して、コードを簡略化・安全性を高めようというバグです。

現在、多くの箇所で、

if (aEvent->eventStructType == NS_KEY_EVENT) {
  foo = static_cast<WidgetKeyboardEvent*>(aEvent)->mBar;
}

といった形でイベントのサブクラスのメンバにアクセスしています。このケースでは、まだ問題が少ないのですが、共通する基底クラスがある場合に処理をするような場合、大きな問題があります。例えば、

switch (aEvent->eventStructType) {
  case NS_INPUT_EVENT:
  case NS_KEY_EVENT:
  case NS_MOUSE_EVENT:
  case NS_DRAG_EVENT:
  case NS_TOUCH_EVENT:
  case NS_SIMPLE_GESTURE_EVENT:
    foo = static_cast<WidgetInputEvent*>(aEvent)->mBar;
    break;
  default:
    break;
}

このようなコードが各所にあった場合、WidgetInputEventを継承するイベントが増えた場合、全てのこのような箇所を修正しなくてはいけないため、修正漏れが簡単に発生します。また、このうちの一部のイベントが、WidgetInputEventを継承しなくなった場合、非常に危険なバグになり得る可能性もありますし、無意味な値を返すだけの分かり難いバグを産むことになります。

こういった問題を解決するのが、As*Eventです。このコードは、以下のように書き換えることができます。

WidgetInputEvent* inputEvent = aEvent->AsInputEvent();
if (inputEvent) {
  foo = inputEvent->mBar;
}

この場合、イベントクラス側での仕様変更で、ハンドリングしている側に影響が出ることはありません。また、スーパークラスの型を前提にしているコードであっても、適合しなくなった場合、nullptrが返され、クラッシュという形で簡単にバグを発見することができるようになります。

逆に言うと、このバグの修正が原因で、クラッシュバグが新たに発生している場合、このバグの修正にミスが無かった場合には、現在リリースされているGeckoのバグが発見されたことを意味している上に、そのバグは危険なものかもしれませんので、報告する際には注意してください。

このAs*Eventメソッドは、virtualメソッドとして実装されているため、デストラクタもvirtualになりました。このため、delete static_cast<WidgetInputEvent*>(mEvent)のようなコードは不要になり、かなり簡略化されています。