chromeos: Add operations to monitor the screen orientation.

content/browser/sensors/sensors_source_chromeos.cc

Index: content/browser/sensors/sensors_source_chromeos.cc
diff --git a/content/browser/sensors/sensors_source_chromeos.cc b/content/browser/sensors/sensors_source_chromeos.cc
new file mode 100644
index 0000000000000000000000000000000000000000..991054df3caadda121db80ee7db5188a3ea1702d
--- /dev/null
+++ b/content/browser/sensors/sensors_source_chromeos.cc
@@ -0,0 +1,77 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/sensors/sensors_source_chromeos.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "content/browser/browser_thread.h"
+#include "content/browser/sensors/sensors_provider.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "dbus/object_proxy.h"
+
+static const char kSensorsServiceName[] = "org.chromium.Sensors";

[satorux1, 2011/09/12 19:46:38]

D-Bus constants should be defined in system_api.git:

http://git.chromium.org/gitweb/?p=chromiumos/platform/system_api.git;a=blob;f...

If you are in hurry, you can go ahead with TODO(cwolfe): Move the constants to
system_api.git.

[tfarina, 2011/09/12 20:07:12]

static is not necessary here. const has implicit internal linkage, it needs
neither static nor an unnamed namespace.

[cwolfe, 2011/09/12 21:17:36]

Forgot about system_api.

Unfortunately that would require restricting these files to 'chromeos==1', and
adding define guards in the main. Added here as if they were included from
system_api.

Fortunately this is a rush job as in "within a few days", rather than "within a
few hours". I've had too many CLs that took a month to get approved.

+static const char kSensorsPath[] = "/";

[satorux1, 2011/09/12 19:46:38]

Using "/" as  object path is not correct: crosbug.com/20135

I guess the sender object name should be something like /org/chromium/Sensors

Again, if you are in hurry, you can go ahead with a TODO(cwolfe): Fix the object
path.

[cwolfe, 2011/09/12 21:17:36]

Done.

[satorux1, 2011/09/12 21:55:14]

I take it back. You should keep it as "/" for now, if you are in a hurry. Making
it something like /org/chromium/Sensors would make testing with dbus-send
impossible.

Background:

dbus-send sends a signal from "/" and there is no way to change the sender path,
and we have code like this in object_proxy.cc:

  // The signal is not coming from the remote object we are attaching to.       
                                                           
  if (signal->GetPath() != object_path_)
    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;

This code seems to be too restrictive. We should probably change the code to

1) not to check the object path
2) make it customizable

I think 1) is simpler and I can do it right now.

+static const char kSensorsInterface[] = "org.chromium.Sensors";
+static const char kSensorsOrientationChanged[] = "ScreenOrientationChanged";
+
+namespace sensors {
+
+SensorsSourceChromeos::SensorsSourceChromeos() : sensors_proxy_(NULL) {
+}
+
+bool SensorsSourceChromeos::Init() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(BrowserThread::IsMessageLoopValid(BrowserThread::IO));
+
+  dbus::Bus::Options options;
+  options.bus_type = dbus::Bus::SYSTEM;
+  options.connection_type = dbus::Bus::PRIVATE;
+  options.dbus_thread_message_loop_proxy =
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO);
+  bus_ = new dbus::Bus(options);
+
+  sensors_proxy_ = bus_->GetObjectProxy(kSensorsServiceName, kSensorsPath);
+  sensors_proxy_->ConnectToSignal(kSensorsInterface, kSensorsOrientationChanged,
+      base::Bind(&SensorsSourceChromeos::OrientationChangedReceived, this),
+      base::Bind(&SensorsSourceChromeos::OrientationChangedConnected, this));
+  return true;
+}
+
+void SensorsSourceChromeos::Stop() {

[satorux1, 2011/09/12 19:46:38]

You might want to add

DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[cwolfe, 2011/09/12 21:17:36]

Done.

Come to think of it, isn't there a which-thread-to-delete-in trait for
RefCountedThreadSafe? Of course I can't find it now.

+  if (bus_)
+    bus_->ShutdownAndBlock();

[satorux1, 2011/09/12 19:46:38]

Shutdown is a bit tricky. Since you are using the IO thread as the D-Bus thread,
it has to be

bus_->ShutdownOnDBusThreadAndBlock(),

unless Stop() is called on D-Bus thread.

[cwolfe, 2011/09/12 21:17:36]

Done.

+}
+
+SensorsSourceChromeos::~SensorsSourceChromeos() {
+  Stop();
+}
+
+void SensorsSourceChromeos::OrientationChangedReceived(dbus::Signal* signal) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  ScreenOrientation orientation;
+
+  dbus::MessageReader reader(signal);
+  int32 upward;

[satorux1, 2011/09/12 19:46:38]

initialize with 0? Uninitialized variable can be lead a non-deterministic bug if
PopInt32() is buggy.

[cwolfe, 2011/09/12 21:17:36]

Done.

+  if (!reader.PopInt32(&upward)) {
+    LOG(WARNING) << "Orientation changed signal had incorrect parameters.";

[satorux1, 2011/09/12 19:46:38]

Let's add << signal->ToString() in the warning. That'll make debugging easier.

[cwolfe, 2011/09/12 21:17:36]

Done.

+    return;
+  }
+  orientation.upward = static_cast<ScreenOrientation::Side>(upward);
+
+  Provider::GetInstance()->ScreenOrientationChanged(orientation);
+}
+
+void SensorsSourceChromeos::OrientationChangedConnected(
+    const std::string& iface_name,
+    const std::string& signal_name,
+    bool success) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!success)
+    LOG(WARNING) << "Failed to connect to orientation changed signal.";
+}
+
+}  // namespace sensors