Closed Bug 1621779 Opened 5 years ago Closed 5 years ago

Fix potential crash in MediaSessionController::UpdateActiveMediaSessionContextId

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox76 --- fixed

People

(Reporter: chunmin, Assigned: chunmin)

References

Details

Attachments

(1 file)

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;
}
Attachment #9132701 - Attachment description: Bug 1621779 - Properly use if-statement in MediaSessionController::UpdateActiveMediaSessionContextId → Bug 1621779 - Correct init-statement within if in UpdateActiveMediaSessionContextId
Attachment #9132701 - Attachment description: Bug 1621779 - Correct init-statement within if in UpdateActiveMediaSessionContextId → Bug 1621779 - Make sure BrowsingContext exists before using it.
Pushed by cbrindusan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/25454b626d6c Make sure BrowsingContext exists before using it. r=alwu
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
See Also: → 1623950
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: