Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 327573002: Properly route screen orientation IPC messages. (Closed)

Created:
6 years, 6 months ago by mlamouri (slow - plz ping)
Modified:
6 years, 6 months ago
Reviewers:
jam, dcheng
CC:
chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, nasko+codewatch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@screen_lock_view
Visibility:
Public.

Description

Properly route screen orientation IPC messages. The messages will now go trough RF/RFH. This is in order to make screen orientation lock Frame specific instead of global to Chromium. The next step is to change the provider accordingly. BUG=162827 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277321 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277755

Patch Set 1 #

Patch Set 2 : fix routing issue + cleanups #

Patch Set 3 : ScreenOrientationDispatcherHost owned by WebContentsImpl #

Total comments: 4

Patch Set 4 : rebase #

Patch Set 5 : #

Patch Set 6 : nit #

Total comments: 2

Patch Set 7 : review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -275 lines) Patch
M content/browser/android/content_view_core_impl.cc View 1 2 3 4 1 chunk +0 lines, -32 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.h View 3 chunks +0 lines, -7 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 6 chunks +0 lines, -11 lines 0 comments Download
M content/browser/screen_orientation/screen_orientation_dispatcher_host.h View 1 2 3 4 5 6 1 chunk +15 lines, -13 lines 0 comments Download
M content/browser/screen_orientation/screen_orientation_dispatcher_host.cc View 1 2 3 4 5 6 3 chunks +21 lines, -12 lines 0 comments Download
M content/browser/screen_orientation/screen_orientation_dispatcher_host_unittest.cc View 1 2 3 4 5 6 1 chunk +0 lines, -184 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 3 4 2 chunks +4 lines, -0 lines 0 comments Download
M content/common/screen_orientation_messages.h View 2 chunks +12 lines, -11 lines 0 comments Download
M content/content_tests.gypi View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M content/renderer/screen_orientation/screen_orientation_dispatcher.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M content/renderer/screen_orientation/screen_orientation_dispatcher_unittest.cc View 6 chunks +12 lines, -2 lines 0 comments Download

Messages

Total messages: 22 (0 generated)
mlamouri (slow - plz ping)
jam@, can you have a look at this CL? :)
6 years, 6 months ago (2014-06-10 17:24:16 UTC) #1
mlamouri (slow - plz ping)
Daniel, could you have a look at the changes in: content/common/screen_orientation_messages.h
6 years, 6 months ago (2014-06-10 17:25:18 UTC) #2
jam
a WebContentsObserver in content should be owned by WebContentsImpl, and not RenderFrameHostImpl. see GeolocationDispatcherHost as ...
6 years, 6 months ago (2014-06-10 18:09:13 UTC) #3
mlamouri (slow - plz ping)
I've updated the CL to make the ScreenOrientationDispatcherHost owned by WebContentsImpl. I guess the design ...
6 years, 6 months ago (2014-06-10 20:50:24 UTC) #4
jam
lgtm https://codereview.chromium.org/327573002/diff/40001/content/browser/screen_orientation/screen_orientation_dispatcher_host.h File content/browser/screen_orientation/screen_orientation_dispatcher_host.h (right): https://codereview.chromium.org/327573002/diff/40001/content/browser/screen_orientation/screen_orientation_dispatcher_host.h#newcode18 content/browser/screen_orientation/screen_orientation_dispatcher_host.h:18: // ScreenOrientationDispatcherHost is receives lock and unlock requests ...
6 years, 6 months ago (2014-06-11 00:42:49 UTC) #5
dcheng
content/common/screen_orientation_messages.h rslgtm
6 years, 6 months ago (2014-06-13 17:04:41 UTC) #6
mlamouri (slow - plz ping)
The CQ bit was checked by mlamouri@chromium.org
6 years, 6 months ago (2014-06-13 17:14:18 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/327573002/40001
6 years, 6 months ago (2014-06-13 17:15:51 UTC) #8
mlamouri (slow - plz ping)
The CQ bit was checked by mlamouri@chromium.org
6 years, 6 months ago (2014-06-13 17:34:19 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/327573002/60001
6 years, 6 months ago (2014-06-13 17:35:33 UTC) #10
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-14 02:43:28 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: android_clang_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/builds/152137)
6 years, 6 months ago (2014-06-14 02:43:29 UTC) #12
mlamouri (slow - plz ping)
The CQ bit was checked by mlamouri@chromium.org
6 years, 6 months ago (2014-06-15 12:49:01 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/327573002/80001
6 years, 6 months ago (2014-06-15 12:49:36 UTC) #14
commit-bot: I haz the power
Change committed as 277321
6 years, 6 months ago (2014-06-15 16:01:50 UTC) #15
tkent
A revert of this CL has been created in https://codereview.chromium.org/336153002/ by tkent@chromium.org. The reason for ...
6 years, 6 months ago (2014-06-16 01:30:18 UTC) #16
mlamouri (slow - plz ping)
Oups, I just realised that I forgot to answer jam@'s comments. Sorry about that. https://codereview.chromium.org/327573002/diff/40001/content/browser/screen_orientation/screen_orientation_dispatcher_host.h ...
6 years, 6 months ago (2014-06-16 14:11:54 UTC) #17
jam
https://codereview.chromium.org/327573002/diff/100001/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc File content/browser/screen_orientation/screen_orientation_dispatcher_host.cc (right): https://codereview.chromium.org/327573002/diff/100001/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc#newcode55 content/browser/screen_orientation/screen_orientation_dispatcher_host.cc:55: Send(new ScreenOrientationMsg_LockError( I missed this last time, you'll need ...
6 years, 6 months ago (2014-06-16 19:20:40 UTC) #18
mlamouri (slow - plz ping)
https://codereview.chromium.org/327573002/diff/100001/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc File content/browser/screen_orientation/screen_orientation_dispatcher_host.cc (right): https://codereview.chromium.org/327573002/diff/100001/content/browser/screen_orientation/screen_orientation_dispatcher_host.cc#newcode55 content/browser/screen_orientation/screen_orientation_dispatcher_host.cc:55: Send(new ScreenOrientationMsg_LockError( On 2014/06/16 19:20:40, jam wrote: > I ...
6 years, 6 months ago (2014-06-17 10:13:57 UTC) #19
mlamouri (slow - plz ping)
The CQ bit was checked by mlamouri@chromium.org
6 years, 6 months ago (2014-06-17 10:14:02 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/327573002/120001
6 years, 6 months ago (2014-06-17 10:16:24 UTC) #21
commit-bot: I haz the power
6 years, 6 months ago (2014-06-17 14:41:58 UTC) #22
Message was sent while issue was closed. 
Change committed as 277755

Powered by Google App Engine
This is Rietveld 408576698