Copy renderer preferences from embedder to guest.

content/browser/browser_plugin/browser_plugin_embedder.cc

 // Copyright (c) 2012 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/browser_plugin/browser_plugin_embedder.h"
 
 #include <set>
 
 #include "base/logging.h"
 #include "base/stl_util.h"
@@ skipping 79 matching lines @@
         render_view_host->GetSiteInstance()->GetSite().host();
     guest_web_contents = WebContentsImpl::CreateGuest(
         web_contents()->GetBrowserContext(),
         host,
         instance_id);
 
     guest = guest_web_contents->GetBrowserPluginGuest();
     guest->set_embedder_render_process_host(
         render_view_host->GetProcess());
 
-    guest_web_contents->GetMutableRendererPrefs()->
-        throttle_input_events = false;
+    RendererPreferences* guest_renderer_prefs =
+        guest_web_contents->GetMutableRendererPrefs();
+    // Copy renderer preferences to guest. For GTK and Aura this is necessary to

[Charlie Reis, 2012/09/27 19:01:32]

I'd like to make a note here that this is intentionally the sole thing we're
copying from the embedder's TabContents, so that we remember where it came from
(in case we decide we're missing something later).  Something as simple as
changing this to:

Copy renderer preferences (and nothing else) from the embedder's TabContents to
the guest.

[lazyboy, 2012/09/27 23:02:23]

Done.

+    // get proper renderer configuration values for caret blinking interval,
+    // colors related to selection and focus.
+    *guest_renderer_prefs = *web_contents()->GetMutableRendererPrefs();

[Charlie Reis, 2012/09/25 19:15:25]

Just curious why this is necessary.  Where does the embedder's web_contents()
have its prefs assigned?  I'm not sure why the guest's WebContents isn't using
the same approach.

[lazyboy, 2012/09/25 19:26:20]

I believe RenderViewHostImpl.CreateRenderView() gets the preferences:

delegate_->GetRendererPrefs(GetProcess()->GetBrowserContext())

For guests, I don't know why they don't default to the same values as the
embedder.

(Win and OSX doesn't seem to care about these values).


On 2012/09/25 19:15:25, creis wrote:

[Charlie Reis, 2012/09/25 21:04:36]

That's probably where the prefs are passed to the new renderer, but it doesn't
explain why the WebContents (i.e., delegate_) has the right settings in the
embedder but not in the guest.

Sorry to push on this, but I'm worried that we might be missing some other
initialization of guest WebContentses if we just paper over this.  Can you find
where the embedder WebContents is getting the correct values from?

[lazyboy, 2012/09/25 21:21:38]

For the embedder, the TabContents's PrefsTabHelper is responsible for retrieving
these settings from system. (that's in chrome/ui).
The guest's WC doesn't have a TabContents. From Fady: Because BrowserPlugin was
designed to operate entirely in the content layer (as per jam@'s suggestions).

On 2012/09/25 21:04:36, creis wrote:

[Charlie Reis, 2012/09/25 23:19:25]

Ah, now we're getting somewhere.  It's interesting that the guest WebContents
has no TabContents.  On the one hand, that's the right thing to do, since
there's no visible, actual tab for it.  On the other hand, there's tons of
things in there that the guest will presumably be missing: ZoomController,
FindTabHelper, etc.

Are we ok with just doing the prefs piecemeal like this and missing everything
else in the guest?  Or are we going to gradually have to add things here as we
realize more is broken?  (Maybe some/most of the stuff in TabContents won't make
sense in a guest, and maybe it won't be a problem as TabContents goes away in
http://crbug.com/107201.)

+
+    guest_renderer_prefs->throttle_input_events = false;
     AddGuest(instance_id, guest_web_contents, frame_id);
     guest_web_contents->SetDelegate(guest);
   } else {
     guest_web_contents =
         static_cast<WebContentsImpl*>(guest->GetWebContents());
   }
 
   // We ignore loading empty urls in web_contents.
   // If a guest sets empty src attribute after it has navigated to some
   // non-empty page, the action is considered no-op.
@@ skipping 123 matching lines @@
       bool visible = *Details<bool>(details).ptr();
       WebContentsVisibilityChanged(visible);
       break;
     }
     default:
       NOTREACHED() << "Unexpected notification type: " << type;
   }
 }
 
 }  // namespace content