Fix for crash in testWindows test

android_webview/browser/browser_view_renderer_impl.cc

 // Copyright (c) 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 "android_webview/browser/browser_view_renderer_impl.h"
 
 #include <android/bitmap.h>
 #include <sys/system_properties.h>
 
 #include "android_webview/common/renderer_picture_map.h"
@@ skipping 25 matching lines @@
 #ifndef GL_TEXTURE_BINDING_EXTERNAL_OES
 #define GL_TEXTURE_BINDING_EXTERNAL_OES 0x8D67
 #endif
 
 using base::android::AttachCurrentThread;
 using base::android::JavaRef;
 using base::android::ScopedJavaLocalRef;
 using content::Compositor;
 using content::ContentViewCore;
 
+namespace android_webview {
+
 namespace {
 
 // Provides software rendering functions from the Android glue layer.
 // Allows preventing extra copies of data when rendering.
 AwDrawSWFunctionTable* g_sw_draw_functions = NULL;
 
 // Tells if the Skia library versions in Android and Chromium are compatible.
 // If they are then it's possible to pass Skia objects like SkPictures to the
 // Android glue layer via the SW rendering functions.
 // If they are not, then additional copies and rasterizations are required
@@ skipping 37 matching lines @@
   }
 
   if (AndroidBitmap_unlockPixels(env, jbitmap.obj()) < 0) {
     LOG(ERROR) << "Error unlocking java bitmap pixels.";
     return false;
   }
 
   return succeeded;
 }
 
+const void* kUserDataKey = &kUserDataKey;
+
 }  // namespace
 
-namespace android_webview {
+class BrowserViewRendererImpl::UserData : public content::WebContents::Data {
+ public:
+  UserData(BrowserViewRendererImpl* ptr) : instance_(ptr) {}
+  virtual ~UserData() {
+    instance_->WebContentsGone();
+  }
+
+  static BrowserViewRendererImpl* GetInstance(content::WebContents* contents) {
+    if (!contents)
+      return NULL;
+    UserData* data = reinterpret_cast<UserData*>(
+        contents->GetUserData(kUserDataKey));
+    return data ? data->instance_ : NULL;
+  }
+
+ private:
+  BrowserViewRendererImpl* instance_;
+};
 
 // static
 BrowserViewRendererImpl* BrowserViewRendererImpl::Create(
     BrowserViewRenderer::Client* client,
     JavaHelper* java_helper) {
   return new BrowserViewRendererImpl(client, java_helper);
 }
 
+// static
+BrowserViewRendererImpl* BrowserViewRendererImpl::FromWebContents(
+    content::WebContents* contents) {
+  return UserData::GetInstance(contents);
+}
+
 BrowserViewRendererImpl::BrowserViewRendererImpl(
     BrowserViewRenderer::Client* client,
     JavaHelper* java_helper)
     : client_(client),
       java_helper_(java_helper),
       view_renderer_host_(new ViewRendererHost(NULL, this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(compositor_(Compositor::Create(this))),
       view_clip_layer_(cc::Layer::create()),
       transform_layer_(cc::Layer::create()),
       scissor_clip_layer_(cc::Layer::create()),
@@ skipping 27 matching lines @@
 void BrowserViewRendererImpl::SetAwDrawSWFunctionTable(
     AwDrawSWFunctionTable* table) {
   g_sw_draw_functions = table;
   g_is_skia_version_compatible =
       g_sw_draw_functions->is_skia_version_compatible(&SkGraphics::GetVersion);
   LOG_IF(WARNING, !g_is_skia_version_compatible)
       << "Skia versions are not compatible, rendering performance will suffer.";
 }
 
 void BrowserViewRendererImpl::SetContents(ContentViewCore* content_view_core) {
+  // First remove association from the prior ContentViewCore / WebContents.
   if (web_contents_) {
     ContentViewCore* previous_content_view_core =
         ContentViewCore::FromWebContents(web_contents_);
     if (previous_content_view_core) {
       previous_content_view_core->RemoveFrameInfoCallback(
           update_frame_info_callback_);
     }
+    web_contents_->SetUserData(kUserDataKey, NULL);
+    DCHECK(!web_contents_);  // WebContentsGone should have been called.
   }
 
-  web_contents_ = content_view_core ?
-                  content_view_core->GetWebContents() : NULL;
-  view_renderer_host_->Observe(web_contents_);
-
   if (!content_view_core)
     return;
 
+  web_contents_ = content_view_core->GetWebContents();
+  web_contents_->SetUserData(kUserDataKey, new UserData(this));
+  view_renderer_host_->Observe(web_contents_);
+
   content_view_core->AddFrameInfoCallback(update_frame_info_callback_);
   dpi_scale_ = content_view_core->GetDpiScale();
 
   view_clip_layer_->removeAllChildren();
   view_clip_layer_->addChild(content_view_core->GetLayer());
   Invalidate();
 }
 
+void BrowserViewRendererImpl::WebContentsGone() {
+  web_contents_ = NULL;

[Leandro Graciá Gil, 2013/03/11 12:07:54]

Should we un-observe in view_renderer_host_?

[joth, 2013/03/11 16:18:31]

No, not really. view_renderer_host_ is a WebContentsObserver so it automatically
gets unobserved when the WebContents departs.
My instinct is this would all be clearer if we hold onto content_view_core_ as a
member instead of web contents, as that's what's set as our input, but that's
going to be a bigger change again so I'm leaving it out for now (as this is a 
flaky test fixer)

+}
+
 void BrowserViewRendererImpl::DrawGL(AwDrawGLInfo* draw_info) {
   TRACE_EVENT0("android_webview", "BrowserViewRendererImpl::DrawGL");
 
   if (view_size_.IsEmpty() || !scissor_clip_layer_ ||
       draw_info->mode == AwDrawGLInfo::kModeProcess)
     return;
 
   DCHECK_EQ(draw_info->mode, AwDrawGLInfo::kModeDraw);
 
   SetCompositorVisibility(view_visible_);
@@ skipping 432 matching lines @@
 
 void BrowserViewRendererImpl::OnFrameInfoUpdated(
     const gfx::SizeF& content_size,
     const gfx::Vector2dF& scroll_offset,
     float page_scale_factor) {
   page_scale_ = page_scale_factor;
   content_size_css_ = content_size;
 }
 
 }  // namespace android_webview