wip: Improving GestureNav screenshotting performance

content/browser/frame_host/navigation_entry_screenshot_manager.cc

 // Copyright 2013 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/frame_host/navigation_entry_screenshot_manager.h"
 
 #include "base/command_line.h"
 #include "base/threading/worker_pool.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/public/browser/overscroll_configuration.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/common/content_switches.h"
-#include "third_party/skia/include/core/SkCanvas.h"
-#include "third_party/skia/include/core/SkPaint.h"
-#include "third_party/skia/include/effects/SkLumaColorFilter.h"
 #include "ui/gfx/codec/png_codec.h"
 
 namespace {
 
 // Minimum delay between taking screenshots.
 const int kMinScreenshotIntervalMS = 1000;
 
 }
 
 namespace content {
@@ skipping 17 matching lines @@
 
   scoped_refptr<base::RefCountedBytes> data() const { return data_; }
 
  private:
   friend class base::RefCountedThreadSafe<ScreenshotData>;
   virtual ~ScreenshotData() {
   }
 
   void EncodeOnWorker(const SkBitmap& bitmap) {
     std::vector<unsigned char> data;
-    // Paint |bitmap| to a kA8_Config SkBitmap
-    SkBitmap a8Bitmap;
-    a8Bitmap.setConfig(SkBitmap::kA8_Config,
-                       bitmap.width(),
-                       bitmap.height(),
-                       0);
-    a8Bitmap.allocPixels();
-    SkCanvas canvas(a8Bitmap);
-    SkPaint paint;
-    SkColorFilter* filter = SkLumaColorFilter::Create();
-    paint.setColorFilter(filter);
-    filter->unref();
-    canvas.drawBitmap(bitmap, SkIntToScalar(0), SkIntToScalar(0), &paint);
-    // Encode the a8Bitmap to grayscale PNG treating alpha as color intensity
-    if (gfx::PNGCodec::EncodeA8SkBitmap(a8Bitmap, &data))
+    // Encode the A8 bitmap to grayscale PNG treating alpha as color intensity.

[danakj, 2014/07/10 19:13:02]

you can dcheck the bitmap config here if you like? might be nice

[mfomitchev, 2014/07/11 19:46:07]

Done. I did it in the other place because the stack trace from that error would
be more useful (since this one is run on WorkerPool), but yeah, I guess that's
not a great reason.

+    if (gfx::PNGCodec::EncodeA8SkBitmap(bitmap, &data))
       data_ = new base::RefCountedBytes(data);
   }
 
   scoped_refptr<base::RefCountedBytes> data_;
 
   DISALLOW_COPY_AND_ASSIGN(ScreenshotData);
 };
 
 NavigationEntryScreenshotManager::NavigationEntryScreenshotManager(
     NavigationControllerImpl* owner)
@@ skipping 46 matching lines @@
         owner_->GetEntryAtIndex(i)));
   }
   DCHECK_EQ(GetScreenshotCount(), 0);
 }
 
 void NavigationEntryScreenshotManager::TakeScreenshotImpl(
     RenderViewHost* host,
     NavigationEntryImpl* entry) {
   DCHECK(host && host->GetView());
   DCHECK(entry);
-  SkBitmap::Config preferred_format = host->PreferredReadbackFormat();
   host->CopyFromBackingStore(
       gfx::Rect(),
       host->GetView()->GetViewBounds().size(),
       base::Bind(&NavigationEntryScreenshotManager::OnScreenshotTaken,
                  screenshot_factory_.GetWeakPtr(),
                  entry->GetUniqueID()),
-      preferred_format);
+     SkBitmap::kA8_Config);
 }
 
 void NavigationEntryScreenshotManager::SetMinScreenshotIntervalMS(
     int interval_ms) {
   DCHECK_GE(interval_ms, 0);
   min_screenshot_interval_ms_ = interval_ms;
 }
 
 void NavigationEntryScreenshotManager::OnScreenshotTaken(int unique_id,
                                                      bool success,
                                                      const SkBitmap& bitmap) {
+  LOG(ERROR) << "OnScreenshotTaken";
+
   NavigationEntryImpl* entry = NULL;
   int entry_count = owner_->GetEntryCount();
   for (int i = 0; i < entry_count; ++i) {
     NavigationEntry* iter = owner_->GetEntryAtIndex(i);
     if (iter->GetUniqueID() == unique_id) {
       entry = NavigationEntryImpl::FromNavigationEntry(iter);
       break;
     }
   }
 
   if (!entry) {
     LOG(ERROR) << "Invalid entry with unique id: " << unique_id;
     return;
   }
 
   if (!success || bitmap.empty() || bitmap.isNull()) {
     if (!ClearScreenshot(entry))
       OnScreenshotSet(entry);
     return;
   }
 
+  CHECK(bitmap.getConfig() == SkBitmap::kA8_Config)

[danakj, 2014/07/10 19:13:02]

DCHECK_EQ, then you don't need the extra << either as it'll print out both

[mfomitchev, 2014/07/11 19:46:07]

Done.

+      << "Incorrect screenshot format: " << bitmap.getConfig();
+
   scoped_refptr<ScreenshotData> screenshot = new ScreenshotData();
   screenshot->EncodeScreenshot(
       bitmap,
       base::Bind(&NavigationEntryScreenshotManager::OnScreenshotEncodeComplete,
                  screenshot_factory_.GetWeakPtr(),
                  unique_id,
                  screenshot));
 }
 
 int NavigationEntryScreenshotManager::GetScreenshotCount() const {
@@ skipping 100 matching lines @@
         owner_->GetEntryAtIndex(forward));
     if (ClearScreenshot(entry))
       --screenshot_count;
     ++forward;
   }
   CHECK_GE(screenshot_count, 0);
   CHECK_LE(screenshot_count, kMaxScreenshots);
 }
 
 }  // namespace content