Merge ~santoshbit2007/oxide:scrollbar_work into oxide:master

Proposed by Santosh
Status: Needs review
Proposed branch: ~santoshbit2007/oxide:scrollbar_work
Merge into: oxide:master
Diff against target: 368 lines (+195/-3)
10 files modified
shared/BUILD.gn (+4/-0)
shared/browser/oxide_content_browser_client.cc (+6/-1)
shared/browser/scrollbar_type.cc (+37/-0)
shared/browser/scrollbar_type.h (+27/-0)
shared/browser/web_contents_helper.cc (+9/-0)
shared/common/oxide_messages.h (+2/-0)
shared/renderer/oxide_content_renderer_client.cc (+7/-2)
shared/renderer/oxide_content_renderer_client.h (+2/-0)
shared/renderer/oxide_scrollbar_state.cc (+54/-0)
shared/renderer/oxide_scrollbar_state.h (+47/-0)
Reviewer Review Type Date Requested Status
Chris Coulson Needs Fixing
Review via email: mp+311234@code.launchpad.net

Description of the change

Implement scrollbar switch when shell mode is changed.

When shell mode is window mode it shows desktop style scrollbar in mobile
Switching happens dynamically.

To post a comment you must log in.
Revision history for this message
Santosh (santoshbit2007) wrote :

overlay scrollbar is controlled by cmd flags --enable-overlay-scrollbar(disable). Disabling overlay scrollbar using cmd require renderer process restart. so I am not changing command line parameter at runtime instead using the blink Runtime setting to disable overlay scrollbar which is enough to disable the overlayscrollbar from blink.
But outside blink chromium checks for cmd params to find type of scrollbar.
So internal of blink may know overlay scrollbar is disabled but at the same time outside of blink
may see its enabled.

But as of now its all fine, as outside blink there is only one place of checking of "type of scrollbar" which is overridden in oxide_content_render_client::OverrideCompositorSettings.

So now ui::IsOverlayScrollbarEnabled(); will return wrong value when convergence is in picture.

Revision history for this message
Chris Coulson (chrisccoulson) wrote :

Thanks, I've left some comments inline.

Also, this appears to still rely on GetFormFactor() to add --enable-overlay-scrollbar to the commandline, but I really want to get rid of that (bug 1548304).

And I'd prefer not to expose ShellMode to the renderer side - I'd rather we just exposed the desired scrollbar configuration

review: Needs Fixing
Revision history for this message
Santosh (santoshbit2007) :

Unmerged commits

37f64b2... by Santosh

Implement scrollbar switch when shell mode is changed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/shared/BUILD.gn b/shared/BUILD.gn
2index c847f04..3976892 100644
3--- a/shared/BUILD.gn
4+++ b/shared/BUILD.gn
5@@ -456,6 +456,8 @@ component("shared") {
6 "browser/screen.h",
7 "browser/screen_observer.cc",
8 "browser/screen_observer.h",
9+ "browser/scrollbar_type.cc",
10+ "browser/scrollbar_type.h",
11 "browser/shell_mode.h",
12 "browser/ssl/oxide_certificate_error.cc",
13 "browser/ssl/oxide_certificate_error.h",
14@@ -556,6 +558,8 @@ component("shared") {
15 "renderer/oxide_v8_scoped_persistent.h",
16 "renderer/oxide_web_content_settings_client.cc",
17 "renderer/oxide_web_content_settings_client.h",
18+ "renderer/oxide_scrollbar_state.cc",
19+ "renderer/oxide_scrollbar_state.h"
20 ]
21
22 if (is_linux) {
23diff --git a/shared/browser/oxide_content_browser_client.cc b/shared/browser/oxide_content_browser_client.cc
24index 876b1ad..7cd6777 100644
25--- a/shared/browser/oxide_content_browser_client.cc
26+++ b/shared/browser/oxide_content_browser_client.cc
27@@ -45,6 +45,7 @@
28 #include "shared/browser/ssl/oxide_certificate_error_dispatcher.h"
29 #include "shared/common/oxide_constants.h"
30 #include "shared/common/oxide_content_client.h"
31+#include "shared/common/oxide_messages.h"
32
33 #include "display_form_factor.h"
34 #include "in_process_renderer_observer.h"
35@@ -60,6 +61,7 @@
36 #include "oxide_user_agent_settings.h"
37 #include "oxide_web_contents_view.h"
38 #include "screen.h"
39+#include "scrollbar_type.h"
40 #include "shell_mode.h"
41 #include "web_contents_client.h"
42 #include "web_contents_helper.h"
43@@ -100,6 +102,7 @@ void ContentBrowserClient::RenderProcessWillLaunch(
44 }
45
46 host->AddFilter(new RenderMessageFilter(host));
47+ host->Send(new OxideMsg_ChangeScrollbarType(IsScrollbarTypeOverlay()));
48 }
49
50 void ContentBrowserClient::SiteInstanceGotProcess(
51@@ -304,11 +307,14 @@ void ContentBrowserClient::OverrideWebkitPrefs(
52 prefs->shrinks_viewport_contents_to_fit = true;
53 prefs->viewport_enabled = true;
54 prefs->main_frame_resizes_are_orientation_changes = true;
55+ prefs->use_solid_color_scrollbars = true;
56 } else {
57 prefs->shrinks_viewport_contents_to_fit = false;
58 prefs->default_minimum_page_scale_factor = 1.0f;
59 prefs->viewport_enabled = false;
60 prefs->main_frame_resizes_are_orientation_changes = false;
61+ // Aura(desktop) overlay scrollbar never use solid color scrollbars
62+ prefs->use_solid_color_scrollbars = false;
63 }
64
65 prefs->supports_multiple_windows = false;
66@@ -317,7 +323,6 @@ void ContentBrowserClient::OverrideWebkitPrefs(
67 contents_helper->client()->CanCreateWindows();
68 }
69
70- prefs->use_solid_color_scrollbars = ui::IsOverlayScrollbarEnabled();
71 }
72
73 content::DevToolsManagerDelegate*
74diff --git a/shared/browser/scrollbar_type.cc b/shared/browser/scrollbar_type.cc
75new file mode 100644
76index 0000000..4f7feba
77--- /dev/null
78+++ b/shared/browser/scrollbar_type.cc
79@@ -0,0 +1,37 @@
80+// vim:expandtab:shiftwidth=2:tabstop=2:
81+// Copyright (C) 2016 Canonical Ltd.
82+
83+// This library is free software; you can redistribute it and/or
84+// modify it under the terms of the GNU Lesser General Public
85+// License as published by the Free Software Foundation; either
86+// version 2.1 of the License, or (at your option) any later version.
87+
88+// This library is distributed in the hope that it will be useful,
89+// but WITHOUT ANY WARRANTY; without even the implied warranty of
90+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
91+// Lesser General Public License for more details.
92+
93+// You should have received a copy of the GNU Lesser General Public
94+// License along with this library; if not, write to the Free Software
95+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
96+
97+#include "shared/browser/scrollbar_type.h"
98+
99+#include "ui/native_theme/native_theme_switches.h"
100+
101+#include "screen.h"
102+#include "shell_mode.h"
103+
104+namespace oxide {
105+
106+bool IsScrollbarTypeOverlay() {
107+ bool is_overlay = ui::IsOverlayScrollbarEnabled();
108+ ShellMode mode = Screen::GetInstance()->GetShellMode();
109+ if (mode == ShellMode::Windowed) {
110+ is_overlay = false;
111+ }
112+
113+ return is_overlay;
114+}
115+
116+} // namespace oxide
117diff --git a/shared/browser/scrollbar_type.h b/shared/browser/scrollbar_type.h
118new file mode 100644
119index 0000000..171a5ab
120--- /dev/null
121+++ b/shared/browser/scrollbar_type.h
122@@ -0,0 +1,27 @@
123+// vim:expandtab:shiftwidth=2:tabstop=2:
124+// Copyright (C) 2016 Canonical Ltd.
125+
126+// This library is free software; you can redistribute it and/or
127+// modify it under the terms of the GNU Lesser General Public
128+// License as published by the Free Software Foundation; either
129+// version 2.1 of the License, or (at your option) any later version.
130+
131+// This library is distributed in the hope that it will be useful,
132+// but WITHOUT ANY WARRANTY; without even the implied warranty of
133+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
134+// Lesser General Public License for more details.
135+
136+// You should have received a copy of the GNU Lesser General Public
137+// License along with this library; if not, write to the Free Software
138+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
139+
140+#ifndef _OXIDE_SHARED_BROWSER_SCROLLBAR_TYPE_H_
141+#define _OXIDE_SHARED_BROWSER_SCROLLBAR_TYPE_H_
142+
143+namespace oxide {
144+
145+bool IsScrollbarTypeOverlay();
146+
147+} // namespace oxide
148+
149+#endif
150diff --git a/shared/browser/web_contents_helper.cc b/shared/browser/web_contents_helper.cc
151index ebcead1..8c0130a 100644
152--- a/shared/browser/web_contents_helper.cc
153+++ b/shared/browser/web_contents_helper.cc
154@@ -21,18 +21,21 @@
155
156 #include "base/lazy_instance.h"
157 #include "base/logging.h"
158+#include "content/public/browser/render_process_host.h"
159 #include "content/public/browser/render_view_host.h"
160 #include "content/public/browser/web_contents.h"
161 #include "content/public/common/renderer_preferences.h"
162 #include "ui/display/display.h"
163
164 #include "shared/common/oxide_content_client.h"
165+#include "shared/common/oxide_messages.h"
166
167 #include "oxide_browser_context.h"
168 #include "oxide_user_agent_settings.h"
169 #include "oxide_web_contents_unloader.h"
170 #include "oxide_web_contents_view.h"
171 #include "oxide_web_view.h"
172+#include "scrollbar_type.h"
173 #include "web_preferences.h"
174
175 namespace oxide {
176@@ -126,6 +129,12 @@ void WebContentsHelper::OnDisplayPropertiesChanged(
177
178 void WebContentsHelper::OnShellModeChanged() {
179 SyncWebPreferences();
180+ for (content::RenderProcessHost::iterator rph_iter(
181+ content::RenderProcessHost::AllHostsIterator());
182+ !rph_iter.IsAtEnd(); rph_iter.Advance()) {
183+ rph_iter.GetCurrentValue()->Send(
184+ new OxideMsg_ChangeScrollbarType(IsScrollbarTypeOverlay()));
185+ }
186 }
187
188 // static
189diff --git a/shared/common/oxide_messages.h b/shared/common/oxide_messages.h
190index 0375b9b..d4cb12d 100644
191--- a/shared/common/oxide_messages.h
192+++ b/shared/common/oxide_messages.h
193@@ -43,6 +43,8 @@ IPC_MESSAGE_CONTROL1(OxideMsg_UpdateUserScripts,
194 IPC_MESSAGE_CONTROL1(OxideMsg_SetUserAgent,
195 std::string)
196
197+IPC_MESSAGE_CONTROL1(OxideMsg_ChangeScrollbarType, bool)
198+
199 IPC_MESSAGE_ROUTED1(OxideMsg_SendMessage,
200 oxide::ScriptMessageParams)
201
202diff --git a/shared/renderer/oxide_content_renderer_client.cc b/shared/renderer/oxide_content_renderer_client.cc
203index 934c19b..063f51b 100644
204--- a/shared/renderer/oxide_content_renderer_client.cc
205+++ b/shared/renderer/oxide_content_renderer_client.cc
206@@ -40,6 +40,7 @@
207 #include "browser_controls_handler.h"
208 #include "oxide_renderer_user_agent_settings.h"
209 #include "oxide_script_message_dispatcher_renderer.h"
210+#include "oxide_scrollbar_state.h"
211 #include "oxide_user_script_scheduler.h"
212 #include "oxide_user_script_slave.h"
213 #include "oxide_web_content_settings_client.h"
214@@ -60,7 +61,10 @@ void ContentRendererClient::RenderThreadStarted() {
215
216 user_agent_settings_.reset(new RendererUserAgentSettings());
217 thread->AddObserver(user_agent_settings_.get());
218-
219+
220+ scrollbar_state_.reset(new ScrollbarState());
221+ thread->AddObserver(scrollbar_state_.get());
222+
223 new UserScriptSlave();
224
225 net::NetModule::SetResourceProvider(NetResourceProvider);
226@@ -149,7 +153,8 @@ void ContentRendererClient::OverrideCompositorSettings(
227 // be able to have 2 different overlay scrollbar styles (a small
228 // non-hit-tested style for mobile and a much larger hit-tested style
229 // for desktop)
230- if (ui::IsOverlayScrollbarEnabled()) {
231+ if (ui::IsOverlayScrollbarEnabled() &&
232+ (scrollbar_state_->IsScrollbarTypeOverlay())) {
233 // XXX: This will need updating if we support overlay scrollbars on
234 // desktop. See https://launchpad.net/bugs/1426567
235 settings->scrollbar_animator = cc::LayerTreeSettings::LINEAR_FADE;
236diff --git a/shared/renderer/oxide_content_renderer_client.h b/shared/renderer/oxide_content_renderer_client.h
237index 9dbb950..5f971d9 100644
238--- a/shared/renderer/oxide_content_renderer_client.h
239+++ b/shared/renderer/oxide_content_renderer_client.h
240@@ -30,6 +30,7 @@ class RendererMediaPlayerManager;
241 #endif
242
243 class RendererUserAgentSettings;
244+class ScrollbarState;
245
246 class ContentRendererClient : public content::ContentRendererClient {
247 public:
248@@ -59,6 +60,7 @@ class ContentRendererClient : public content::ContentRendererClient {
249 std::string GetUserAgentOverrideForURL(const GURL& url) override;
250
251 std::unique_ptr<RendererUserAgentSettings> user_agent_settings_;
252+ std::unique_ptr<ScrollbarState> scrollbar_state_;
253
254 DISALLOW_COPY_AND_ASSIGN(ContentRendererClient);
255 };
256diff --git a/shared/renderer/oxide_scrollbar_state.cc b/shared/renderer/oxide_scrollbar_state.cc
257new file mode 100644
258index 0000000..e414f1e
259--- /dev/null
260+++ b/shared/renderer/oxide_scrollbar_state.cc
261@@ -0,0 +1,54 @@
262+// vim:expandtab:shiftwidth=2:tabstop=2:
263+// Copyright (C) 2015 Canonical Ltd.
264+
265+// This library is free software; you can redistribute it and/or
266+// modify it under the terms of the GNU Lesser General Public
267+// License as published by the Free Software Foundation; either
268+// version 2.1 of the License, or (at your option) any later version.
269+
270+// This library is distributed in the hope that it will be useful,
271+// but WITHOUT ANY WARRANTY; without even the implied warranty of
272+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
273+// Lesser General Public License for more details.
274+
275+// You should have received a copy of the GNU Lesser General Public
276+// License along with this library; if not, write to the Free Software
277+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
278+
279+#include "shared/renderer/oxide_scrollbar_state.h"
280+
281+#include "third_party/WebKit/public/web/WebRuntimeFeatures.h"
282+#include "third_party/WebKit/public/web/WebView.h"
283+
284+#include "base/command_line.h"
285+#include "content/public/renderer/render_frame.h"
286+#include "ipc/ipc_message_macros.h"
287+#include "ui/native_theme/native_theme_switches.h"
288+
289+#include "shared/common/oxide_constants.h"
290+#include "shared/common/oxide_messages.h"
291+
292+namespace oxide {
293+
294+ScrollbarState::ScrollbarState() {
295+ scrollbar_type_overlay_ = ui::IsOverlayScrollbarEnabled();
296+}
297+
298+ScrollbarState::~ScrollbarState() {}
299+
300+bool ScrollbarState::OnControlMessageReceived(const IPC::Message& message) {
301+ bool handled = true;
302+ IPC_BEGIN_MESSAGE_MAP(ScrollbarState, message)
303+ IPC_MESSAGE_HANDLER(OxideMsg_ChangeScrollbarType, OnChangeScrollbarType)
304+ IPC_MESSAGE_UNHANDLED(handled = false)
305+ IPC_END_MESSAGE_MAP()
306+
307+ return handled;
308+}
309+
310+void ScrollbarState::OnChangeScrollbarType(bool is_overlay) {
311+ scrollbar_type_overlay_ = is_overlay;
312+ blink::WebRuntimeFeatures::enableOverlayScrollbars(is_overlay);
313+}
314+
315+} // namespace oxide
316diff --git a/shared/renderer/oxide_scrollbar_state.h b/shared/renderer/oxide_scrollbar_state.h
317new file mode 100644
318index 0000000..f643d3f
319--- /dev/null
320+++ b/shared/renderer/oxide_scrollbar_state.h
321@@ -0,0 +1,47 @@
322+// vim:expandtab:shiftwidth=2:tabstop=2:
323+// Copyright (C) 2015 Canonical Ltd.
324+
325+// This library is free software; you can redistribute it and/or
326+// modify it under the terms of the GNU Lesser General Public
327+// License as published by the Free Software Foundation; either
328+// version 2.1 of the License, or (at your option) any later version.
329+
330+// This library is distributed in the hope that it will be useful,
331+// but WITHOUT ANY WARRANTY; without even the implied warranty of
332+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
333+// Lesser General Public License for more details.
334+
335+// You should have received a copy of the GNU Lesser General Public
336+// License along with this library; if not, write to the Free Software
337+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
338+
339+#ifndef _OXIDE_SHARED_RENDERER_SHELL_MODE_STATE_H_
340+#define _OXIDE_SHARED_RENDERER_SHELL_MODE_STATE_H_
341+
342+#include "content/public/renderer/render_thread_observer.h"
343+
344+#include "shared/browser/shell_mode.h"
345+
346+namespace oxide {
347+
348+class ScrollbarState : public content::RenderThreadObserver {
349+ public:
350+ ScrollbarState();
351+ ~ScrollbarState() override;
352+
353+ bool IsScrollbarTypeOverlay() const { return scrollbar_type_overlay_; }
354+
355+ private:
356+ // RenderThreadObserver implementation
357+ bool OnControlMessageReceived(const IPC::Message& message) override;
358+
359+ void OnChangeScrollbarType(bool is_overlay);
360+
361+ bool scrollbar_type_overlay_;
362+
363+ DISALLOW_COPY_AND_ASSIGN(ScrollbarState);
364+};
365+
366+} // namespace oxide
367+
368+#endif // _OXIDE_SHARED_RENDERER_SHELL_MODE_STATE_H_

Subscribers

People subscribed via source and target branches

to all changes: