Stub for ARC print Bridge

chrome/browser/chromeos/arc/arc_print_bridge.cc

+// Copyright 2016 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 "chrome/browser/chromeos/arc/arc_print_bridge.h"
+
+#include <utility>
+
+#include "ash/common/shell_delegate.h"
+#include "ash/common/wm_shell.h"
+#include "ash/new_window_delegate.h"
+#include "ash/shell.h"
+#include "base/files/file.h"
+#include "base/files/scoped_file.h"
+#include "base/logging.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_checker.h"
+#include "components/arc/arc_bridge_service.h"
+#include "content/public/browser/browser_thread.h"
+#include "mojo/edk/embedder/embedder.h"
+#include "url/gurl.h"
+
+namespace arc {
+
+ArcPrintBridge::ArcPrintBridge(ArcBridgeService* bridge_service)
+    : ArcService(bridge_service), binding_(this) {
+  arc_bridge_service()->AddObserver(this);

[Luis Héctor Chávez, 2016/07/14 16:29:43]

arc_bridge_service()->print()->AddObserver(this);

[Sergey Poromov, 2016/07/19 16:32:09]

Done.

+}
+
+ArcPrintBridge::~ArcPrintBridge() {
+  arc_bridge_service()->RemoveObserver(this);
+}
+
+void ArcPrintBridge::OnPrintInstanceReady() {
+  mojom::PrintInstance* print_instance = arc_bridge_service()->print_instance();
+  if (!print_instance) {
+    LOG(ERROR) << "OnPrintInstanceReady called, but no print instance found";
+    return;
+  }
+
+  print_instance->Init(binding_.CreateInterfacePtrAndBind());
+}
+
+void ArcPrintBridge::Print(mojo::ScopedHandle file) {
+  if (!file.is_valid()) {
+    LOG(ERROR) << "handle is invalid";
+    return;
+  }
+
+  mojo::edk::ScopedPlatformHandle scoped_platform_handle;
+  MojoResult mojo_result = mojo::edk::PassWrappedPlatformHandle(
+      file.release().value(), &scoped_platform_handle);
+  if (mojo_result != MOJO_RESULT_OK) {
+    LOG(ERROR) << "PassWrappedPlatformHandle failed: " << mojo_result;
+    return;
+  }
+
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ArcPrintBridge::SavePdf, this,
+                 scoped_platform_handle.release().handle),

[Luis Héctor Chávez, 2016/07/14 16:29:43]

Ideally, make the base::ScopedFD conversion in this method:

base::ScopedFD fd(scoped_platform_handle.release().handle);

and then pass it into the base::Bind with

base::Passed(&fd)

otherwise, you'll leak the fd in some cases.

[Sergey Poromov, 2016/07/19 16:32:09]

Done.

+      base::Bind(&ArcPrintBridge::OpenPdf, this));
+}
+
+void ArcPrintBridge::SavePdf(int handle) {

[Luis Héctor Chávez, 2016/07/14 16:29:43]

This doesn't really need to be a member function. Put it in the anonymous
namespace and make it return the RAII-wrapped base::FilePath. That way you also
avoid the need to make this class RefCounted.

Doing it that way also has the additional benefit of supporting multiple
concurrent print jobs, which is not the case right now.

For the RAII-wrapped class you can do something like this:

class ScopedTempFile {
 public:
  ScopedTempFile() {
    base::CreateTemporaryFile(&file_path_);
  }
  ~ScopedTempFile() {
    base::DeleteFile(file_path_, false);
  }

  const base::FilePath& file_path() const { return file_path_; }

 private:
  base::FilePath file_path_;
  DISALLOW_COPY_AND_ASSIGN(ScopedTempFile);
};

[Sergey Poromov, 2016/07/19 16:32:09]

Done.

+  base::ScopedFD fd(handle);

[Luis Héctor Chávez, 2016/07/14 16:29:44]

DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);

[Sergey Poromov, 2016/07/19 16:32:09]

Done.

+
+  base::CreateTemporaryFile(&file_path_);
+  base::File out(file_path_,
+                 base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);

[Luis Héctor Chávez, 2016/07/14 16:29:43]

CreateTemporaryfile() creates the file to avoid races. Is FLAG_CREATE_ALWAYS
needed?

[Sergey Poromov, 2016/07/19 16:32:09]

Yes, it doesn't work without this flag.

+
+  char buf[8192];
+  size_t bytes;
+  while ((bytes = HANDLE_EINTR(read(fd.get(), buf, 8192))) > 0) {
+    out.WriteAtCurrentPos(buf, bytes);
+  }
+}
+
+void ArcPrintBridge::OpenPdf() {

[Luis Héctor Chávez, 2016/07/14 16:29:43]

This method should receive the RAII-wrapped base::FilePath as parameter and
delete the file once it's done printing. Otherwise you'll fill up the user's
disk with old print jobs.

[Sergey Poromov, 2016/07/19 16:32:09]

Unfortunately we couldn't delete the file here right now - it will be not
available from new opened Chrome tab.
This implementation is just temporary and won't be exposed to users - we won't
switch from Android printing to this one while this is not fully implemented.
However, for development and testing it's better to have file here so that we
can try to do something with the data on Chrome side.

+  GURL gurl("file://" + file_path_.value());

[Luis Héctor Chávez, 2016/07/14 16:29:43]

Can you also use the constants in
https://cs.chromium.org/chromium/src/url/url_constants.h instead?

[Luis Héctor Chávez, 2016/07/14 16:29:44]

DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);

[Sergey Poromov, 2016/07/19 16:32:09]

Done.

[Sergey Poromov, 2016/07/19 16:32:09]

Done.

+  ash::WmShell::Get()->delegate()->OpenUrl(gurl);
+}
+
+}  // namespace arc