1621779 - Fix potential crash in MediaSessionController::UpdateActiveMediaSessionContextId

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[C.M.Chang[:chunmin]]

In MediaSessionController::UpdateActiveMediaSessionContextId,

if (RefPtr<BrowsingContext> bc = BrowsingContext::Get(iter.Key());
        bc->IsTopContent())

bc->IsTopContent() will cause a crash if bc is nullptr (can be reprodiced by replacing iter.Key() by 0xdeaddead).

The reason is that the RefPtr<BrowsingContext> bc = BrowsingContext::Get(iter.Key()); will be evaluate to a statement (it has a ;), instead of a condition, so bc->... will still be executed whether bc is NULL or not.

I am not a C++ language expert, but the online C++ IDE gives the same result: https://repl.it/repls/WindyPlumpEquation

#include <iostream>
#include <memory>

class X {
 public:
  bool is_positive() { return value > 0; }
  X(int v = 0) : value(v) {}

 private:
  int value;
};

int main() {
  if (std::unique_ptr<X> ptr = nullptr) { // will be evaluated as a condition
    std::cout << "this won't be printed" << std::endl;
  } else {
    std::cout << "print this" << std::endl;
  }

  if (std::unique_ptr<X> ptr(nullptr); ptr->is_positive()) {
    std::cout << "Never reach here. Crash at `ptr->...` since `ptr` is `NULL`!"
              << std::endl;
  }

  return 0;
}

Comment 1

[C.M.Chang[:chunmin]]

Attachment: Bug 1621779 - Make sure BrowsingContext exists before using it.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25454b626d6c
Make sure BrowsingContext exists before using it. r=alwu

Comment 3

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/25454b626d6c