Firefox 62で修正されたFirefox 61での後退バグに見る、不要なコードを削除する事の大切さ - 2018-06-07 - ククログ

ククログ

株式会社クリアコード > ククログ > Firefox 62で修正されたFirefox 61での後退バグに見る、不要なコードを削除する事の大切さ

Firefox 62で修正されたFirefox 61での後退バグに見る、不要なコードを削除する事の大切さ

一般的に、プログラムの継続的な開発においては「機能の追加」や「不具合の修正」が行われる事は多いですが、「機能の削除」が明示的に行われる事はそう多くないのではないでしょうか1。この度、使われなくなっていた機能が残っていた事により意図しない所で不具合が発生し、機能を削除することで不具合が解消された事例がありましたので、使われなくなった機能を削除する事の意義を示す一時例としてご紹介いたします。

発生していた現象

現在のFirefoxでは、インストールするアドオンは必ずMozillaによる電子署名が施されていなくてはならず、未署名のファイルからはアドオンをインストールできないようになっています。ただ、それではアドオンの開発そのものができないため、開発者向けの機能であるabout:debuggingから、開発中のアドオンを「一時的なアドオン」として読み込んで動作させられるようになっています。

この機能について、Firefox 61で「条件によっては、アドオンを一時的なアドオンとして読み込めない」「条件によっては、一時的なアドオンの読み込みに非常に時間がかかる」という不具合が発生していましたが、Firefox 62で修正される事になりました2

現象が起こり始めたきっかけと、原因の調査

開発中のアドオンを一時的なアドオンとして読み込めないという現象について、現象発生のタイミングを二分探索で絞り込むツールであるmozregressionを使って調査した所、1452827 - XPIInstall has a bunch of cruft that needs to be cleaned upでの変更が投入されて以降発生するようになっていた事が分かりました。

また、この現象が発生していた場面では、WSL3で作成したシンボリックリンクがリポジトリ内に含まれている場合に、開発者用のコンソールにwinLastError: 1920という情報が出力されるという状況でした。これはWindowsが返すファイル関連のエラーコードのひとつであるERROR_CANT_ACCESS_FILEを意味する物で、Bugzilla上において、上記変更の中に含まれているファイルの情報を取得する処理がWSLのシンボリックリンクに対してこのエラーを報告している、という情報を頂く事ができました。

実際の修正

ファイルの情報を調べようとして何らかのエラーが発生するファイルというものは、WSLのシンボリックリンク以外にも存在し得ます。また、一般的に、規模が大きな変更は予想外の影響をもたらすリスクがあり、変更は可能な限り最小限に留める事が望ましいとされます。これらの理由から、当初は「ファイルの情報を取得しようとした時のエラーを適切にハンドルすることで、特殊なファイルがあっても問題が起こらないようにする」という方向でパッチを作成し提出しました。

しかしながら、このパッチはレビューの結果却下され、そもそもこの「ファイルの情報を取得する処理」自体が不要だからそれを削除する方がよいのではないかという指摘を受けました。そこで改めて調査したところ、以下の状況である事を確認できました。

  • この処理は、アドオンの総ファイルサイズを計算する過程で呼ばれている。

  • しかしながら、現行のFirefoxではアドオンのファイルサイズは画面上に表示される事が無く、また、ファイルサイズの情報が有効に使われる部分も存在していない。

    • 過去には使われていた機能のようだが、機能の改廃が進んだ結果、現在は「ファイルサイズの計算が正しいかどうか」を検証する自動テストの中で参照されるだけの機能になっていた。

    • 類似の情報として、自動更新を通じてダウンロードされたアドオンのインストールパッケージのファイルサイズという物もある。

      • こちらは現在も使われているが、今回問題となっている「アドオンの総ファイルサイズ」とは別の物である。
  • よって、アドオンの総ファイルサイズという情報自体が現在では無用となっている。

既にある機能や情報の削除は、その機能を使っている箇所が1つでも残っていれば不可能なため、影響範囲の調査は特に念入りに行う必要があります。今回の事例では、

  • Mozillaのスタッフの人から、使われていない機能を削除する方向での対応が望ましい旨のコメントが出ていた。

  • 実際に調査した限りでは、確かに目に見える部分や他の機能からは参照されていないという事の確認が取れた。

という2つの根拠があったため、思い切って、アドオンの総ファイルサイズの計算に関わるコードと、サイズ情報を参照している箇所4を削除するという内容でパッチを再作成しました。その結果、パッチは無事に取り込まれ、Firefox 62ではこの問題が修正される事になりました。

また、このパッチが取り込まれた結果、「条件によって一時的なアドオンの読み込みに非常に時間がかかる」という別のBugにも影響が及んでいた事が後になって分かりました。こちらのBugは、開発中のアドオンのフォルダー配下にnode_modulesのようなフォルダーがあると5、その配下の大量のファイルをスキャンするために時間がかかるようになってしまった、という物です。アドオンの総ファイルサイズの計算自体を行わなくした事により、棚ぼた的にこちらのBugも解消されたという状況でした。

ただ、実はその後、「WSLのシンボリックリンクのせいでエラーが発生してアドオンを読み込めない」という状況自体が今となってはほぼ発生しなくなっているという事も分かりました。具体的には、Bugの報告時にエラーの原因になっていたシンボリックリンクはWindows 10 Creators UpdateやWindows 10 Fall Creators UpdateのWSLで作成された物でしたが、その後Windows 10 April 2018 UpdateのWSLなどではNTFSの妥当なシンボリックリンクが作成されるようWSLが改良されたため、この問題の影響を受けるのは「古いWSLで(npm installを実行するなどして間接的に作成される場合も含めて)シンボリックリンクを作って、それをその後もずっと使い続けている場合」だけという事になっています。一般ユーザーに影響が及ばないアドオン開発者向けの機能であるという事に加え、プラットフォーム側の改善により今では問題自体が発生しなくなっている(新たにシンボリックリンクを作り直すだけでよい)という事も相まって、このパッチはFirefox 61には取り込まれないという判断がなされています。

まとめ

以上、Firefox 62で取り込まれたパッチを題材に、既に不要となっていた機能が問題を引き起こしていた事例についてご紹介しました。

Firefox 56およびそれ以前のバージョンのFirefoxにおいて使用できていた従来型アドオンは、今回削除されたコードのような「Firefox内部では既に使われなくなった機能」を使用している事が度々ありました。そのためFirefox全体として、アドオンが動作しなくなる事を警戒して古いコードを大量に抱え込まざるを得ない状態が長く続いていました。Firefox57での「従来型アドオン(XULアドオン)廃止」という大きな変化には、このような状態を是正するためという意味合いもあったと言えるでしょう。これを反面教師として、不要になった機能や実装はその都度速やかに削除してコードを簡潔に保っていくという事の重要さを実感していただければ幸いです。

  1. ただし、機能一式の刷新にあたって「新しい実装に持ち越されなかった機能」が結果的に「旧バージョンから削除された機能」として見える、という事はあります。

  2. 一般ユーザーへの影響は小さいと判断された結果、Firefox 61への修正の反映は見送られています。

  3. Windows Subsystem for Linux

  4. 大半は自動テスト内で、UI上に現れる物もFirefoxでは使われていないThunderbird用に残されたコードに1箇所あるだけでした。

  5. 静的な文法チェックを行うためにeslintを使っている場合などに、このような状況が発生し得ます。