Should WebContentsObserver::web_contents() be public?

[Václav Brožek]

Hi all, especially //content OWNERS.

During a recent review (https://codereview.chromium.org/512683003/), I pushed back against promoting the protected WebContentsObserver::web_contents() to public in a class inheriting the WebContentsObserver.

It was pointed out to me, that there are many instances of this. Randomly pointing at DevToolsWindow::ObserverWithAccessor::GetWebContents(), DownloadRequestLimiter::TabDownloadState::web_contents(), but there are much more (look for "return web_contents()" or promoting through "using" in the Codesearch).

If it is OK for random classes inheriting the WebContentsObserver to expose web_contents(), then instead WebContentsObserver::web_contents() should be made public. If it is not, then there are cases to be fixed. In both situations, there is some clean-up to do.

(This was also filed as http://crbug.com/408560, to capture related code changes.)

Cheers,

Vaclav

[John Abd-El-Malek]

I agree, thanks for bringing this up.

I originally did this because I thought it might be an implementation detail of a WCO. However as you point out, in practice many needed to expose it. When we added RenderFrameObserver, we quickly exposed a RenderFrame* getter there. So exposing web_contents() sgtm.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev