From e449c33540bb37833d11b85a4f7f7008902aa7c5 Mon Sep 17 00:00:00 2001
From: "davemoore@chromium.org"
 <davemoore@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Sat, 30 Oct 2010 23:22:26 +0000
Subject: Fix race condition resulting in crash after taking photo

BUG=chromium-os:8440
TEST=Create new user, take photo...ensure that chrome didn't crash
     either by checking the pids or looking at /var/log/session_manager.

Review URL: http://codereview.chromium.org/4164013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64556 0039d316-1c4b-4281-b951-d872f2087c98
---
 chrome/browser/chromeos/login/camera.cc | 28 +++++++++++++++++++++++++++-
 chrome/browser/chromeos/login/camera.h  |  6 ++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

(limited to 'chrome')

diff --git a/chrome/browser/chromeos/login/camera.cc b/chrome/browser/chromeos/login/camera.cc
index 18d6ce3..1fb85f8e 100644
--- a/chrome/browser/chromeos/login/camera.cc
+++ b/chrome/browser/chromeos/login/camera.cc
@@ -152,6 +152,7 @@ Camera::~Camera() {
   DCHECK_EQ(-1, device_descriptor_) << "Don't forget to uninitialize camera.";
 }
 
+// If this method is called there's no need to call PostCameraThreadAck().
 void Camera::ReportFailure() {
   DCHECK(IsOnCameraThread());
   if (device_descriptor_ == -1) {
@@ -190,6 +191,7 @@ void Camera::DoInitialize(int desired_width, int desired_height) {
 
   if (device_descriptor_ != -1) {
     LOG(WARNING) << "Camera is initialized already.";
+    PostCameraThreadAck();
     return;
   }
 
@@ -245,12 +247,24 @@ void Camera::DoInitialize(int desired_width, int desired_height) {
   frame_height_ = frame_size.height();
   desired_width_ = desired_width;
   desired_height_ = desired_height;
+  // No need to call PostCameraThreadAck() back as this method
+  // is being posted instead.
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       NewRunnableMethod(this, &Camera::OnInitializeSuccess));
 }
 
+void Camera::CameraThreadAck() {
+}
+
+void Camera::PostCameraThreadAck() {
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      NewRunnableMethod(this, &Camera::CameraThreadAck));
+}
+
 void Camera::Uninitialize() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   PostCameraTask(NewRunnableMethod(this, &Camera::DoUninitialize));
@@ -260,6 +274,7 @@ void Camera::DoUninitialize() {
   DCHECK(IsOnCameraThread());
   if (device_descriptor_ == -1) {
     LOG(WARNING) << "Calling uninitialize for uninitialized camera.";
+    PostCameraThreadAck();
     return;
   }
   DoStopCapturing();
@@ -267,6 +282,8 @@ void Camera::DoUninitialize() {
   if (close(device_descriptor_) == -1)
     log_errno("Closing the device failed.");
   device_descriptor_ = -1;
+  // Maintain a reference so that camera object isn't deleted on wrong thread.
+  PostCameraThreadAck();
 }
 
 void Camera::StartCapturing() {
@@ -278,6 +295,7 @@ void Camera::DoStartCapturing() {
   DCHECK(IsOnCameraThread());
   if (is_capturing_) {
     LOG(WARNING) << "Capturing is already started.";
+    PostCameraThreadAck();
     return;
   }
 
@@ -298,6 +316,8 @@ void Camera::DoStartCapturing() {
     ReportFailure();
     return;
   }
+  // No need to post DidProcessCameraThreadMethod() as this method is
+  // being posted instead.
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
@@ -315,6 +335,7 @@ void Camera::StopCapturing() {
 void Camera::DoStopCapturing() {
   DCHECK(IsOnCameraThread());
   if (!is_capturing_) {
+    PostCameraThreadAck();
     LOG(WARNING) << "Calling StopCapturing when capturing is not started.";
     return;
   }
@@ -323,6 +344,8 @@ void Camera::DoStopCapturing() {
   v4l2_buf_type type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
   if (xioctl(device_descriptor_, VIDIOC_STREAMOFF, &type) == -1)
     log_errno("VIDIOC_STREAMOFF failed.");
+  // Maintain a reference so that camera object isn't deleted on wrong thread.
+  PostCameraThreadAck();
 }
 
 void Camera::GetFrame(SkBitmap* frame) {
@@ -407,8 +430,11 @@ void Camera::UnmapVideoBuffers() {
 
 void Camera::OnCapture() {
   DCHECK(IsOnCameraThread());
-  if (!is_capturing_)
+  if (!is_capturing_) {
+    // Maintain a reference so that camera object isn't deleted on wrong thread.
+    PostCameraThreadAck();
     return;
+  }
 
   do {
     fd_set fds;
diff --git a/chrome/browser/chromeos/login/camera.h b/chrome/browser/chromeos/login/camera.h
index 38e7e19..af70e62 100644
--- a/chrome/browser/chromeos/login/camera.h
+++ b/chrome/browser/chromeos/login/camera.h
@@ -128,6 +128,12 @@ class Camera : public base::RefCountedThreadSafe<Camera> {
   void DoUninitialize();
   void DoStopCapturing();
 
+  // All methods on the camera thread need to call this to post back to the UI
+  // thread. Otherwise the camera object could be deleted on the camera thread
+  // which is not allowed.
+  void PostCameraThreadAck();
+  void CameraThreadAck();
+
   // Returns true if the code is executed on camera thread right now, false
   // otherwise.
   bool IsOnCameraThread() const;
-- 
cgit v1.1