Block incognito browser windows for voice interaction.

chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc

Index: chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
diff --git a/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc b/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
index c0c1c9d135badab625e71d8ca9b499d6b6d97839..e9cf9ae65e730dea87f4b30dc4504575262e5e60 100644
--- a/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
+++ b/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
@@ -4,21 +4,34 @@
 
 #include "chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.h"
 
+#include <set>
 #include <utility>
 #include <vector>
 
 #include "ash/accelerators/accelerator_controller.h"
 #include "ash/shell.h"
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/task_scheduler/post_task.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chromeos/chromeos_switches.h"
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/instance_holder.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/aura/window.h"
+#include "ui/compositor/layer.h"
+#include "ui/compositor/layer_owner.h"
+#include "ui/compositor/layer_tree_owner.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_util.h"
+#include "ui/gfx/native_widget_types.h"
 #include "ui/snapshot/snapshot.h"
+#include "ui/snapshot/snapshot_aura.h"
+#include "ui/wm/core/window_util.h"
 #include "ui/wm/public/activation_client.h"
 
 namespace arc {
@@ -37,6 +50,43 @@ void ScreenshotCallback(
   callback.Run(result);
 }
 
+std::unique_ptr<ui::LayerTreeOwner> BlockIncognitoWindows(
+    aura::Window* root_window) {
+  using LayerSet = std::set<const ui::Layer*>;
+  auto blocked_layers = LayerSet();

[Luis Héctor Chávez, 2017/05/15 16:47:52]

Why not

LayerSet blocked_layers;

? It's shorter and more readable.

[Muyuan, 2017/05/15 18:09:54]

Done.

+  auto* browser_list = BrowserList::GetInstance();
+  for (auto* browser : *browser_list) {

[Luis Héctor Chávez, 2017/05/15 16:47:51]

nit: BrowserList itself does

for (auto* browser : *BrowserList::GetInstance())

https://cs.chromium.org/chromium/src/chrome/browser/ui/browser_list.cc?l=31

[Muyuan, 2017/05/15 18:09:54]

Done.

+    if (browser->profile()->IsOffTheRecord())
+      blocked_layers.insert(browser->window()->GetNativeWindow()->layer());
+  }
+
+  auto layer_tree_owner = ::wm::MapLayers(
+      root_window, base::Bind(
+                       [](LayerSet layers,
+                          ui::LayerOwner* owner) -> std::unique_ptr<ui::Layer> {
+                         // Parent layer is excluded meaning that it's pointless
+                         // to clone current child and all its descendants. This
+                         // reduces the number of layers to create.
+                         if (layers.count(owner->layer()->parent()))
+                           return nullptr;
+                         if (layers.count(owner->layer())) {
+                           auto layer = base::MakeUnique<ui::Layer>(
+                               ui::LayerType::LAYER_SOLID_COLOR);
+                           layer->SetBounds(owner->layer()->bounds());
+                           layer->SetColor(SK_ColorBLACK);
+                           return layer;
+                         }
+                         return owner->RecreateLayer();
+                       },
+                       std::move(blocked_layers)));
+  if (layer_tree_owner) {

[Luis Héctor Chávez, 2017/05/15 16:47:52]

nit: use the guard clause pattern:

if (!layer_tree_owner)
  return layer_tree_owner;

// rest

[Muyuan, 2017/05/15 18:09:54]

Done.

+    auto* root_layer = layer_tree_owner->root();
+    root_window->layer()->Add(root_layer);
+    root_window->layer()->StackAtBottom(root_layer);
+  }
+  return layer_tree_owner;
+}
+
 }  // namespace
 
 // static
@@ -136,11 +186,14 @@ void ArcVoiceInteractionFrameworkService::CaptureFullscreen(
   // the screenshot to it.
   aura::Window* window = ash::Shell::GetPrimaryRootWindow();
   DCHECK(window);
-  ui::GrabWindowSnapshotAsyncJPEG(
-      window, gfx::Rect(window->bounds().size()),
-      base::CreateTaskRunnerWithTraits(
-          {base::MayBlock(), base::TaskPriority::USER_BLOCKING}),
-      base::Bind(&ScreenshotCallback, callback));
+
+  old_layer_owner_ = BlockIncognitoWindows(window);

[Luis Héctor Chávez, 2017/05/15 16:47:52]

It doesn't seem like you *need* to store this as a member. How about sending it
with base::Passed(...) to
ArcVoiceInteractionFrameworkService::EncodeAndReturnImage?

+  DCHECK(old_layer_owner_);

[Luis Héctor Chávez, 2017/05/15 16:47:52]

I don't think you can DCHECK here since it seems like it's legit that you get a
nullptr from BlockIncognitoWindows.

If it's not legit, you should add more comments/DCHECKs there mentioning why it
should "never happen".

[Muyuan, 2017/05/15 18:09:54]

Done.

+
+  ui::GrabLayerSnapshotAsync(
+      old_layer_owner_->root(), gfx::Rect(window->bounds().size()),
+      base::Bind(&ArcVoiceInteractionFrameworkService::EncodeAndReturnImage,
+                 base::Unretained(this), callback));
 }
 
 void ArcVoiceInteractionFrameworkService::OnMetalayerClosed() {
@@ -196,4 +249,21 @@ void ArcVoiceInteractionFrameworkService::SetMetalayerVisibility(bool visible) {
   framework_instance->SetMetalayerVisibility(visible);
 }
 
+void ArcVoiceInteractionFrameworkService::EncodeAndReturnImage(
+    const CaptureFullscreenCallback& callback,
+    const gfx::Image& image) {
+  old_layer_owner_.reset();
+  base::PostTaskWithTraitsAndReplyWithResult(
+      FROM_HERE,
+      base::TaskTraits({base::MayBlock(), base::TaskPriority::USER_BLOCKING}),

[Luis Héctor Chávez, 2017/05/15 16:47:51]

nit:

base::TaskTraits{base::MayBlock(), base::TaskPriority::USER_BLOCKING}

otherwise you're building a base::TaskTraits with the {...}, and then building
another one with base::TaskTraits(...) and passing the original in.

[Muyuan, 2017/05/15 18:09:54]

Done.

+      base::Bind(
+          [](const gfx::Image& image) -> std::vector<uint8_t> {
+            std::vector<uint8_t> res;
+            gfx::JPEG1xEncodedDataFromImage(image, 100, &res);
+            return res;
+          },
+          image),
+      callback);
+}
+
 }  // namespace arc