Sets a transparent background for out-of-process subframes.

Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/caab514ea9a212c7a0cbdc6a001a86736451679f
Cr-Commit-Position: refs/heads/master@{#377937}

[lfg, 2016-01-06 22:18:26]

Description was changed from

==========
Sets a transparent background for out-of-process subframes.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
==========

to

==========
Sets a transparent background for out-of-process subframes by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
==========

[lfg, 2016-01-06 22:31:51]

Description was changed from

==========
Sets a transparent background for out-of-process subframes by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
==========

to

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
==========

[lfg, 2016-01-21 23:19:07]

Description was changed from

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
==========

to

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[lfg, 2016-01-27 21:13:15]

Patchset #6 (id:100001) has been deleted

[lfg, 2016-01-27 22:14:59]

lfg@chromium.org changed reviewers:
+ dcheng@chromium.org

[lfg, 2016-01-27 22:15:00]

Hey Daniel, please take a look.

[lfg, 2016-01-27 22:15:26]

Patchset #7 (id:140001) has been deleted

[dcheng, 2016-01-27 23:50:08]

How come there's some hittesting changes in this CL?

[lfg, 2016-01-28 00:28:45]

On 2016/01/27 23:50:08, dcheng wrote:
> How come there's some hittesting changes in this CL?

That's to support hittesting on transparent backgrounds. Otherwise,
the hittest goes through the surface coming from the renderer with
the transparent background. I could split the cc/ part of the change
into a separate CL.

[lfg, 2016-02-02 18:18:59]

Hey Daniel, I've split the cc/ changes into a separate patch. Please take
another look.

[lfg, 2016-02-02 18:21:30]

On 2016/02/02 18:18:59, lfg wrote:
> Hey Daniel, I've split the cc/ changes into a separate patch. Please take
> another look.

I've also wrote a short design doc for this:
https://docs.google.com/a/google.com/document/d/1C_oEvGjPyMMZzFX9V-2suy8VXHh9...

[dcheng, 2016-02-10 22:13:44]

https://codereview.chromium.org/1565893004/diff/220001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebFrameWidget.h (right):

https://codereview.chromium.org/1565893004/diff/220001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebFrameWidget.h:74: virtual void
setBackgroundColorOverride(WebColor) = 0;
Does this need to be the in public layer? I don't actually see any calls to this
method through a WebFrameWidget pointer.

The only calls I see to this are in DevToolsEmulator, which lives in Source/web.
So how does this get set? What am I missing?

[lfg, 2016-02-10 23:20:24]

https://codereview.chromium.org/1565893004/diff/220001/third_party/WebKit/pub...
File third_party/WebKit/public/web/WebFrameWidget.h (right):

https://codereview.chromium.org/1565893004/diff/220001/third_party/WebKit/pub...
third_party/WebKit/public/web/WebFrameWidget.h:74: virtual void
setBackgroundColorOverride(WebColor) = 0;
On 2016/02/10 22:13:44, dcheng wrote:
> Does this need to be the in public layer? I don't actually see any calls to
this
> method through a WebFrameWidget pointer.
> 
> The only calls I see to this are in DevToolsEmulator, which lives in
Source/web.
> So how does this get set? What am I missing?

If I remember correctly, when I did this I was planning on converting all calls
to WebView to calls to WebFrameWidget, but ended up forgetting to do it. In that
case we would need this public interface, at least temporarily until
WebFrameWidget/WebViewFrameWidget is disentangled.

I could either remove the public interfaces, convert the call sites to use
WebFrameWidget, or convert the call sites and assume the widgets are
WebViewFrameWidgets. What do you think we should do for now?

[lfg, 2016-02-11 22:34:37]

Hey Daniel, I've removed the override, please take a look.

[dcheng, 2016-02-11 22:40:26]

https://codereview.chromium.org/1565893004/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/1565893004/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:67: if
(localRoot->parent())
Do you think it'd make sense to have this in the ctor? That way, if someone
accidentally calls WebFrameWidgetImpl::create(), they still get the right
default.

https://codereview.chromium.org/1565893004/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:300: 
The WebView version also marks the FrameViews as transparent: do we need
anything like that?

[lfg, 2016-02-11 23:48:02]

https://codereview.chromium.org/1565893004/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/1565893004/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:67: if
(localRoot->parent())
On 2016/02/11 22:40:26, dcheng wrote:
> Do you think it'd make sense to have this in the ctor? That way, if someone
> accidentally calls WebFrameWidgetImpl::create(), they still get the right
> default.

Done.

https://codereview.chromium.org/1565893004/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:300: 
On 2016/02/11 22:40:26, dcheng wrote:
> The WebView version also marks the FrameViews as transparent: do we need
> anything like that?

I believe this is correct. isTransparent() on FrameView only controls painting
the background if it's the main frame (iframes are transparent by default), and
controls things that should be treated differently when the main frame's
background is transparent. In this case, the main frame is not transparent, we
just want to drop the background layer in the local root's widget.

However, we may want to set the frameviews as transparent when the WebViewImpl
in the main frame is set as transparent, but looking at usages of
FrameView::isTransparent(), I don't think we are breaking anything by not
replicating this parameter.

[dcheng, 2016-02-12 22:46:24]

lgtm

https://codereview.chromium.org/1565893004/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/1565893004/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:300: 
On 2016/02/11 at 23:48:02, lfg wrote:
> On 2016/02/11 22:40:26, dcheng wrote:
> > The WebView version also marks the FrameViews as transparent: do we need
> > anything like that?
> 
> I believe this is correct. isTransparent() on FrameView only controls painting
the background if it's the main frame (iframes are transparent by default), and
controls things that should be treated differently when the main frame's
background is transparent. In this case, the main frame is not transparent, we
just want to drop the background layer in the local root's widget.
> 
> However, we may want to set the frameviews as transparent when the WebViewImpl
in the main frame is set as transparent, but looking at usages of
FrameView::isTransparent(), I don't think we are breaking anything by not
replicating this parameter.

Hm... I guess I am thinking that it would make it easier to merge widget
portions of WebView into WebFrameWidget into the future. But I guess at that
point, we'll have to manually diff and reconcile any behavior differences...
maybe a TODO could be useful though.

[lfg, 2016-02-25 17:23:09]

lfg@chromium.org changed reviewers:
+ boliu@chromium.org, lcwu@chromium.org, piman@chromium.org

[lfg, 2016-02-25 17:23:10]

boliu@chromium.org: Please review changes in 
android_webview/*

lcwu@chromium.org: Please review changes in 
chromecast/*

piman@chromium.org: Please review changes in 
content/*

[lcwu1, 2016-02-25 17:28:41]

lcwu@chromium.org changed reviewers:
+ halliwell@chromium.org

[halliwell, 2016-02-25 17:32:28]

On 2016/02/25 17:28:42, lcwu1 wrote:

chromecast/ lgtm

[boliu, 2016-02-25 17:56:03]

Possibly dumb question. I noticed the methods on WebViewImpl aren't actually
removed? Any reason for keeping them?

https://codereview.chromium.org/1565893004/diff/260001/android_webview/render...
File android_webview/renderer/aw_render_frame_ext.cc (right):

https://codereview.chromium.org/1565893004/diff/260001/android_webview/render...
android_webview/renderer/aw_render_frame_ext.cc:257: blink::WebFrameWidget*
webframewidget = GetWebFrameWidget();
nit: web_frame_widget

[piman, 2016-02-25 21:58:37]

rs lgtm if dcheng@ is happy

[lfg, 2016-02-25 22:12:23]

On 2016/02/25 17:56:03, boliu wrote:
> Possibly dumb question. I noticed the methods on WebViewImpl aren't actually
> removed? Any reason for keeping them?

We still use the WebViewImpl as the widget for the main frame. Eventually we
would like to switch everything to use WebFrameWidget, but for now we still
need more work to get to that point.

[lfg, 2016-02-25 22:12:40]

https://codereview.chromium.org/1565893004/diff/260001/android_webview/render...
File android_webview/renderer/aw_render_frame_ext.cc (right):

https://codereview.chromium.org/1565893004/diff/260001/android_webview/render...
android_webview/renderer/aw_render_frame_ext.cc:257: blink::WebFrameWidget*
webframewidget = GetWebFrameWidget();
On 2016/02/25 17:56:03, boliu wrote:
> nit: web_frame_widget

Done.

[boliu, 2016-02-25 22:17:14]

lgtm % compile

https://codereview.chromium.org/1565893004/diff/280001/android_webview/render...
File android_webview/renderer/aw_render_frame_ext.cc (right):

https://codereview.chromium.org/1565893004/diff/280001/android_webview/render...
android_webview/renderer/aw_render_frame_ext.cc:258: if (!webframewidget)
this doesn't compile..

[lfg, 2016-02-26 17:50:52]

The CQ bit was checked by lfg@chromium.org

[lfg, 2016-02-26 17:50:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
halliwell@chromium.org, dcheng@chromium.org, piman@chromium.org,
boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/1565893004/#ps300001
(title: "compile")

[commit-bot: I haz the power, 2016-02-26 17:52:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1565893004/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1565893004/300001

[lfg, 2016-02-26 17:52:24]

Description was changed from

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-02-26 19:06:59]

Description was changed from

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-02-26 19:07:01]

Committed patchset #14 (id:300001)

[commit-bot: I haz the power, 2016-02-26 19:08:15]

Description was changed from

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Sets a transparent background for out-of-process subframes
by default.

This CL moves background color from WebView to WebFrameWidget,
which allows for setting the background color independently per
local root. WebViewImpl still implements background color for
the main frame, but it now goes through a WebViewFrameWidget
interface.

BUG=515625
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/caab514ea9a212c7a0cbdc6a001a86736451679f
Cr-Commit-Position: refs/heads/master@{#377937}
==========

[commit-bot: I haz the power, 2016-02-26 19:08:18]

Patchset 14 (id:??) landed as
https://crrev.com/caab514ea9a212c7a0cbdc6a001a86736451679f
Cr-Commit-Position: refs/heads/master@{#377937}