[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

This CL hides ScreenOrientationProvider inside WebContentsImpl,
because it is not so necessary to be content public.
Instead this CL:
  - Exposes SetScreenOrientationDelegate() on web_contents.h
  - Exposes OnScreenOrientationChange() on web_contents_impl.h

Then it would be quite clear what the touchpoints of ScreenOrientationProvider
with the rest of the browser process are, which makes it all the easier to
analyze them (and to avoid those touchpoints growing larger as we try to
eliminate them).

BUG=678545
TEST=content_unittests
[email protected]

Review-Url: https://codereview.chromium.org/2682643002
Cr-Commit-Position: refs/heads/master@{#449288}
Committed: https://chromium.googlesource.com/chromium/src/+/552e9de9f523cb755187ab7c3a288f25bee4df8b

[leonhsl(Using Gerrit), 2017-02-07 09:06:58]

[email protected] changed reviewers:
+ [email protected], [email protected]

[leonhsl(Using Gerrit), 2017-02-07 09:06:59]

This CL comes from Colin's suggestions at
https://codereview.chromium.org/2649823002/#msg19, PTAL, Thanks.

As this CL depends on https://codereview.chromium.org/2649823002/ which has not
been committed, I put together the 2 CLs as another CL
https://codereview.chromium.org/2685513002/ just for running trybots.

[leonhsl(Using Gerrit), 2017-02-07 14:29:17]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 14:29:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[leonhsl(Using Gerrit), 2017-02-07 14:35:21]

The CQ bit was checked by [email protected] to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 14:35:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-07 15:38:38]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-02-07 15:38:39]

Dry run: This issue passed the CQ dry run.

[blundell, 2017-02-07 16:03:16]

I'll be OOO until Feb 20, but no need to block this CL on me. I leafed through
it and it looks good overall.

[leonhsl(Using Gerrit), 2017-02-08 03:09:09]

Description was changed from

==========
[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

BUG=678545
TEST=content_unittests
==========

to

==========
[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

This CL hides ScreenOrientationProvider inside WebContentsImpl,
because it is not so necessary to be content public.
This CL:
  - Exposes SetScreenOrientationDelegate() on web_contents.h
  - Exposes OnScreenOrientationChange() on web_contents_impl.h

Then it would be quite clear what the touchpoints of ScreenOrientationProvider
with the rest of the browser process are, which makes it all the easier to
analyze them (and to avoid those touchpoints growing larger as we try to
eliminate them).

BUG=678545
TEST=content_unittests
==========

[leonhsl(Using Gerrit), 2017-02-08 03:09:51]

Description was changed from

==========
[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

This CL hides ScreenOrientationProvider inside WebContentsImpl,
because it is not so necessary to be content public.
This CL:
  - Exposes SetScreenOrientationDelegate() on web_contents.h
  - Exposes OnScreenOrientationChange() on web_contents_impl.h

Then it would be quite clear what the touchpoints of ScreenOrientationProvider
with the rest of the browser process are, which makes it all the easier to
analyze them (and to avoid those touchpoints growing larger as we try to
eliminate them).

BUG=678545
TEST=content_unittests
==========

to

==========
[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

This CL hides ScreenOrientationProvider inside WebContentsImpl,
because it is not so necessary to be content public.
Instead this CL:
  - Exposes SetScreenOrientationDelegate() on web_contents.h
  - Exposes OnScreenOrientationChange() on web_contents_impl.h

Then it would be quite clear what the touchpoints of ScreenOrientationProvider
with the rest of the browser process are, which makes it all the easier to
analyze them (and to avoid those touchpoints growing larger as we try to
eliminate them).

BUG=678545
TEST=content_unittests
==========

[leonhsl(Using Gerrit), 2017-02-08 03:12:07]

[email protected] changed reviewers:
+ [email protected], [email protected]

[leonhsl(Using Gerrit), 2017-02-08 03:12:07]

+kinuko@ and rockot@, would you please also take an overall review at this?
Thanks.

[mlamouri (slow - plz ping), 2017-02-08 17:37:33]

lgtm

[kinuko, 2017-02-09 05:57:38]

lgtm

https://codereview.chromium.org/2682643002/diff/40001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.h (right):

https://codereview.chromium.org/2682643002/diff/40001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.h:214: ScreenOrientationProvider*
screen_orientation_provider_for_testing() const {
This name sounds like it's returning mock/testing-only screen orientation
provider or something (as lower_case() inline accessors usually correspond to
lower_case_ variables).  GetScreenOrientationProviderForTesting() would be
clearer?

[leonhsl(Using Gerrit), 2017-02-09 08:32:06]

[email protected] changed reviewers:
+ [email protected]

[leonhsl(Using Gerrit), 2017-02-09 08:32:07]

Thanks all for review! Let me TBR jamescook@ for the trivial change of
ash/content/screen_orientation_delegate_chromeos.cc, will send to CQ now.

https://codereview.chromium.org/2682643002/diff/40001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.h (right):

https://codereview.chromium.org/2682643002/diff/40001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.h:214: ScreenOrientationProvider*
screen_orientation_provider_for_testing() const {
On 2017/02/09 05:57:38, kinuko wrote:
> This name sounds like it's returning mock/testing-only screen orientation
> provider or something (as lower_case() inline accessors usually correspond to
> lower_case_ variables).  GetScreenOrientationProviderForTesting() would be
> clearer?

Got it and done.

[leonhsl(Using Gerrit), 2017-02-09 08:32:58]

Description was changed from

==========
[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

This CL hides ScreenOrientationProvider inside WebContentsImpl,
because it is not so necessary to be content public.
Instead this CL:
  - Exposes SetScreenOrientationDelegate() on web_contents.h
  - Exposes OnScreenOrientationChange() on web_contents_impl.h

Then it would be quite clear what the touchpoints of ScreenOrientationProvider
with the rest of the browser process are, which makes it all the easier to
analyze them (and to avoid those touchpoints growing larger as we try to
eliminate them).

BUG=678545
TEST=content_unittests
==========

to

==========
[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

This CL hides ScreenOrientationProvider inside WebContentsImpl,
because it is not so necessary to be content public.
Instead this CL:
  - Exposes SetScreenOrientationDelegate() on web_contents.h
  - Exposes OnScreenOrientationChange() on web_contents_impl.h

Then it would be quite clear what the touchpoints of ScreenOrientationProvider
with the rest of the browser process are, which makes it all the easier to
analyze them (and to avoid those touchpoints growing larger as we try to
eliminate them).

BUG=678545
TEST=content_unittests
[email protected]
==========

[leonhsl(Using Gerrit), 2017-02-09 08:33:08]

The CQ bit was checked by [email protected]

[leonhsl(Using Gerrit), 2017-02-09 08:33:08]

The patchset sent to the CQ was uploaded after l-g-t-m from [email protected],
[email protected]
Link to the patchset: https://codereview.chromium.org/2682643002/#ps60001
(title: "screen_orientation_provider_for_testing() -->
GetScreenOrientationProviderForTesting()")

[commit-bot: I haz the power, 2017-02-09 08:33:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 10:12:59]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-02-09 10:13:00]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[leonhsl(Using Gerrit), 2017-02-09 10:17:06]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2017-02-09 10:17:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 10:53:17]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2017-02-09 10:53:18]

Failed to apply patch for content/browser/android/content_view_core_impl.cc:
While running git apply --index -p1;
  error: patch failed: content/browser/android/content_view_core_impl.cc:1398
  error: content/browser/android/content_view_core_impl.cc: patch does not apply

Patch:       content/browser/android/content_view_core_impl.cc
Index: content/browser/android/content_view_core_impl.cc
diff --git a/content/browser/android/content_view_core_impl.cc
b/content/browser/android/content_view_core_impl.cc
index
392db7b22efd445ef65ae90fb3b2a9caad6f73e2..248553f7557c44f8b73e49ea70269ecef779baa7
100644
--- a/content/browser/android/content_view_core_impl.cc
+++ b/content/browser/android/content_view_core_impl.cc
@@ -43,7 +43,6 @@
 #include "content/public/browser/favicon_status.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
-#include "content/public/browser/screen_orientation_provider.h"
 #include "content/public/browser/ssl_host_state_delegate.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_client.h"
@@ -1398,9 +1397,7 @@ void
ContentViewCoreImpl::SendOrientationChangeEventInternal() {
   if (rwhv)
     rwhv->UpdateScreenInfo(GetViewAndroid());
 
-  static_cast<WebContentsImpl*>(web_contents())
-      ->GetScreenOrientationProvider()
-      ->OnOrientationChange();
+  static_cast<WebContentsImpl*>(web_contents())->OnScreenOrientationChange();
 }
 
 void ContentViewCoreImpl::ExtractSmartClipData(JNIEnv* env,

[leonhsl(Using Gerrit), 2017-02-09 13:26:29]

The CQ bit was checked by [email protected]

[leonhsl(Using Gerrit), 2017-02-09 13:26:29]

The patchset sent to the CQ was uploaded after l-g-t-m from [email protected],
[email protected]
Link to the patchset: https://codereview.chromium.org/2682643002/#ps80001
(title: "Rebase only")

[commit-bot: I haz the power, 2017-02-09 13:26:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-09 14:37:34]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486646789332680,
"parent_rev": "9f178059a3f0fd6f7a151bfe129a697ceaf0b150", "commit_rev":
"552e9de9f523cb755187ab7c3a288f25bee4df8b"}

[commit-bot: I haz the power, 2017-02-09 14:38:19]

Description was changed from

==========
[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

This CL hides ScreenOrientationProvider inside WebContentsImpl,
because it is not so necessary to be content public.
Instead this CL:
  - Exposes SetScreenOrientationDelegate() on web_contents.h
  - Exposes OnScreenOrientationChange() on web_contents_impl.h

Then it would be quite clear what the touchpoints of ScreenOrientationProvider
with the rest of the browser process are, which makes it all the easier to
analyze them (and to avoid those touchpoints growing larger as we try to
eliminate them).

BUG=678545
TEST=content_unittests
[email protected]
==========

to

==========
[ScreenOrientation] Hide ScreenOrientationProvider inside WebContentsImpl.

This CL hides ScreenOrientationProvider inside WebContentsImpl,
because it is not so necessary to be content public.
Instead this CL:
  - Exposes SetScreenOrientationDelegate() on web_contents.h
  - Exposes OnScreenOrientationChange() on web_contents_impl.h

Then it would be quite clear what the touchpoints of ScreenOrientationProvider
with the rest of the browser process are, which makes it all the easier to
analyze them (and to avoid those touchpoints growing larger as we try to
eliminate them).

BUG=678545
TEST=content_unittests
[email protected]

Review-Url: https://codereview.chromium.org/2682643002
Cr-Commit-Position: refs/heads/master@{#449288}
Committed:
https://chromium.googlesource.com/chromium/src/+/552e9de9f523cb755187ab7c3a28...
==========

[commit-bot: I haz the power, 2017-02-09 14:38:21]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/552e9de9f523cb755187ab7c3a28...

[James Cook, 2017-02-09 15:19:07]

ash lgtm