Merge lp:~osomon/oxide/backport-42-patch-for-isError into lp:oxide/1.5
- backport-42-patch-for-isError
- Merge into 1.5
Proposed by
Olivier Tilloy
Status: | Merged |
---|---|
Merged at revision: | 966 |
Proposed branch: | lp:~osomon/oxide/backport-42-patch-for-isError |
Merge into: | lp:oxide/1.5 |
Diff against target: |
298 lines (+266/-0) 4 files modified
patches/bug1423531.patch (+202/-0) patches/series (+1/-0) qt/tests/qmltests/api/tst_WebView_loading.qml (+23/-0) qt/tests/qmltests/api/tst_WebView_save_restore_state.qml (+40/-0) |
To merge this branch: | bzr merge lp:~osomon/oxide/backport-42-patch-for-isError |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Chris Coulson | Approve | ||
Review via email: mp+251749@code.launchpad.net |
Commit message
Backport commit 7431bb2983db683
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === added file 'patches/bug1423531.patch' |
2 | --- patches/bug1423531.patch 1970-01-01 00:00:00 +0000 |
3 | +++ patches/bug1423531.patch 2015-03-04 14:35:28 +0000 |
4 | @@ -0,0 +1,202 @@ |
5 | +# Description: backport commit 7431bb2983db683dcc4556e1918556254c3d5589 |
6 | +# from the 42 branch to fix https://launchpad.net/bugs/1423531 |
7 | +# Author: Olivier Tilloy <olivier.tilloy@canonical.com> |
8 | + |
9 | +diff --git a/chrome/browser/ui/zoom/zoom_controller_browsertest.cc b/chrome/browser/ui/zoom/zoom_controller_browsertest.cc |
10 | +--- a/chrome/browser/ui/zoom/zoom_controller_browsertest.cc |
11 | ++++ b/chrome/browser/ui/zoom/zoom_controller_browsertest.cc |
12 | +@@ -8,6 +8,7 @@ |
13 | + #include "base/process/kill.h" |
14 | + #include "chrome/browser/profiles/profile.h" |
15 | + #include "chrome/browser/ui/browser.h" |
16 | ++#include "chrome/browser/ui/browser_commands.h" |
17 | + #include "chrome/browser/ui/tabs/tab_strip_model.h" |
18 | + #include "chrome/browser/ui/webui/signin/login_ui_test_utils.h" |
19 | + #include "chrome/browser/ui/zoom/chrome_zoom_level_prefs.h" |
20 | +@@ -140,6 +141,57 @@ |
21 | + EXPECT_FLOAT_EQ(new_zoom_level, zoom_controller->GetZoomLevel()); |
22 | + } |
23 | + |
24 | ++IN_PROC_BROWSER_TEST_F(ZoomControllerBrowserTest, |
25 | ++ ErrorPagesCanZoomAfterTabRestore) { |
26 | ++ // This url is meant to cause a network error page to be loaded. |
27 | ++ // Tests can't reach the network, so this test should continue |
28 | ++ // to work even if the domain listed is someday registered. |
29 | ++ GURL url("http://kjfhkjsdf.com"); |
30 | ++ |
31 | ++ TabStripModel* tab_strip = browser()->tab_strip_model(); |
32 | ++ ASSERT_TRUE(tab_strip); |
33 | ++ |
34 | ++ ui_test_utils::NavigateToURLWithDisposition( |
35 | ++ browser(), url, NEW_FOREGROUND_TAB, |
36 | ++ ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION); |
37 | ++ { |
38 | ++ content::WebContents* web_contents = tab_strip->GetActiveWebContents(); |
39 | ++ |
40 | ++ EXPECT_EQ( |
41 | ++ content::PAGE_TYPE_ERROR, |
42 | ++ web_contents->GetController().GetLastCommittedEntry()->GetPageType()); |
43 | ++ |
44 | ++ content::WebContentsDestroyedWatcher destroyed_watcher(web_contents); |
45 | ++ tab_strip->CloseWebContentsAt(tab_strip->active_index(), |
46 | ++ TabStripModel::CLOSE_CREATE_HISTORICAL_TAB); |
47 | ++ destroyed_watcher.Wait(); |
48 | ++ } |
49 | ++ EXPECT_EQ(1, tab_strip->count()); |
50 | ++ |
51 | ++ content::WebContentsAddedObserver new_web_contents_observer; |
52 | ++ chrome::RestoreTab(browser()); |
53 | ++ content::WebContents* web_contents = |
54 | ++ new_web_contents_observer.GetWebContents(); |
55 | ++ content::WaitForLoadStop(web_contents); |
56 | ++ |
57 | ++ EXPECT_EQ(2, tab_strip->count()); |
58 | ++ |
59 | ++ EXPECT_EQ( |
60 | ++ content::PAGE_TYPE_ERROR, |
61 | ++ web_contents->GetController().GetLastCommittedEntry()->GetPageType()); |
62 | ++ |
63 | ++ ZoomController* zoom_controller = |
64 | ++ ZoomController::FromWebContents(web_contents); |
65 | ++ |
66 | ++ double old_zoom_level = zoom_controller->GetZoomLevel(); |
67 | ++ double new_zoom_level = old_zoom_level + 0.5; |
68 | ++ |
69 | ++ // The following attempt to change the zoom level for an error page should |
70 | ++ // fail. |
71 | ++ zoom_controller->SetZoomLevel(new_zoom_level); |
72 | ++ EXPECT_FLOAT_EQ(new_zoom_level, zoom_controller->GetZoomLevel()); |
73 | ++} |
74 | ++ |
75 | + IN_PROC_BROWSER_TEST_F(ZoomControllerBrowserTest, Observe) { |
76 | + content::WebContents* web_contents = |
77 | + browser()->tab_strip_model()->GetActiveWebContents(); |
78 | +diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc |
79 | +--- a/content/browser/frame_host/navigation_controller_impl.cc |
80 | ++++ b/content/browser/frame_host/navigation_controller_impl.cc |
81 | +@@ -1050,10 +1050,6 @@ |
82 | + pending_entry_->site_instance() == rfh->GetSiteInstance())) { |
83 | + new_entry = new NavigationEntryImpl(*pending_entry_); |
84 | + |
85 | +- // Don't use the page type from the pending entry. Some interstitial page |
86 | +- // may have set the type to interstitial. Once we commit, however, the page |
87 | +- // type must always be normal. |
88 | +- new_entry->set_page_type(PAGE_TYPE_NORMAL); |
89 | + update_virtual_url = new_entry->update_virtual_url_with_url(); |
90 | + } else { |
91 | + new_entry = new NavigationEntryImpl; |
92 | +@@ -1074,8 +1070,12 @@ |
93 | + update_virtual_url = needs_update; |
94 | + } |
95 | + |
96 | +- if (params.url_is_unreachable) |
97 | +- new_entry->set_page_type(PAGE_TYPE_ERROR); |
98 | ++ // Don't use the page type from the pending entry. Some interstitial page |
99 | ++ // may have set the type to interstitial. Once we commit, however, the page |
100 | ++ // type must always be normal or error. |
101 | ++ new_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR |
102 | ++ : PAGE_TYPE_NORMAL); |
103 | ++ |
104 | + new_entry->SetURL(params.url); |
105 | + if (update_virtual_url) |
106 | + UpdateVirtualURLToURL(new_entry, params.url); |
107 | +@@ -1126,6 +1126,8 @@ |
108 | + NavigationEntryImpl* entry = entries_[entry_index].get(); |
109 | + |
110 | + // The URL may have changed due to redirects. |
111 | ++ entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR |
112 | ++ : PAGE_TYPE_NORMAL); |
113 | + entry->SetURL(params.url); |
114 | + entry->SetReferrer(params.referrer); |
115 | + if (entry->update_virtual_url_with_url()) |
116 | +@@ -1178,6 +1180,8 @@ |
117 | + existing_entry->set_unique_id(pending_entry_->GetUniqueID()); |
118 | + |
119 | + // The URL may have changed due to redirects. |
120 | ++ existing_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR |
121 | ++ : PAGE_TYPE_NORMAL); |
122 | + if (existing_entry->update_virtual_url_with_url()) |
123 | + UpdateVirtualURLToURL(existing_entry, params.url); |
124 | + existing_entry->SetURL(params.url); |
125 | +@@ -1204,6 +1208,8 @@ |
126 | + // entry and it will be the same page as the new navigation (minus the |
127 | + // reference fragments, of course). We'll update the URL of the existing |
128 | + // entry without pruning the forward history. |
129 | ++ existing_entry->set_page_type(params.url_is_unreachable ? PAGE_TYPE_ERROR |
130 | ++ : PAGE_TYPE_NORMAL); |
131 | + existing_entry->SetURL(params.url); |
132 | + if (existing_entry->update_virtual_url_with_url()) |
133 | + UpdateVirtualURLToURL(existing_entry, params.url); |
134 | +diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc |
135 | +--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc |
136 | ++++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc |
137 | +@@ -28,6 +28,7 @@ |
138 | + #include "content/public/browser/web_contents_observer.h" |
139 | + #include "content/public/common/content_switches.h" |
140 | + #include "content/public/common/page_state.h" |
141 | ++#include "content/public/common/page_type.h" |
142 | + #include "content/public/common/url_constants.h" |
143 | + #include "content/public/test/mock_render_process_host.h" |
144 | + #include "content/public/test/test_notification_tracker.h" |
145 | +@@ -4456,4 +4457,61 @@ |
146 | + EXPECT_EQ(0, delegate->repost_form_warning_count()); |
147 | + } |
148 | + |
149 | ++TEST_F(NavigationControllerTest, UnreachableURLGivesErrorPage) { |
150 | ++ GURL url("http://foo"); |
151 | ++ FrameHostMsg_DidCommitProvisionalLoad_Params params; |
152 | ++ params.page_id = 1; |
153 | ++ params.url = url; |
154 | ++ params.transition = ui::PAGE_TRANSITION_LINK; |
155 | ++ params.gesture = NavigationGestureUser; |
156 | ++ params.page_state = PageState::CreateFromURL(url); |
157 | ++ params.was_within_same_page = false; |
158 | ++ params.is_post = true; |
159 | ++ params.post_id = 2; |
160 | ++ params.url_is_unreachable = true; |
161 | ++ // Navigate to new page |
162 | ++ { |
163 | ++ LoadCommittedDetails details; |
164 | ++ controller_impl().RendererDidNavigate(main_test_rfh(), params, &details); |
165 | ++ EXPECT_EQ(PAGE_TYPE_ERROR, |
166 | ++ controller_impl().GetLastCommittedEntry()->GetPageType()); |
167 | ++ EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, details.type); |
168 | ++ } |
169 | ++ |
170 | ++ // Navigate to existing page. |
171 | ++ { |
172 | ++ LoadCommittedDetails details; |
173 | ++ controller_impl().RendererDidNavigate(main_test_rfh(), params, &details); |
174 | ++ EXPECT_EQ(PAGE_TYPE_ERROR, |
175 | ++ controller_impl().GetLastCommittedEntry()->GetPageType()); |
176 | ++ EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, details.type); |
177 | ++ } |
178 | ++ |
179 | ++ // Navigate to same page. |
180 | ++ // Note: The call to LoadURL() creates a pending entry in order to trigger the |
181 | ++ // same-page transition. |
182 | ++ controller_impl().LoadURL( |
183 | ++ url, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string()); |
184 | ++ params.transition = ui::PAGE_TRANSITION_TYPED; |
185 | ++ { |
186 | ++ LoadCommittedDetails details; |
187 | ++ controller_impl().RendererDidNavigate(main_test_rfh(), params, &details); |
188 | ++ EXPECT_EQ(PAGE_TYPE_ERROR, |
189 | ++ controller_impl().GetLastCommittedEntry()->GetPageType()); |
190 | ++ EXPECT_EQ(NAVIGATION_TYPE_SAME_PAGE, details.type); |
191 | ++ } |
192 | ++ |
193 | ++ // Navigate in page. |
194 | ++ params.url = GURL("http://foo#foo"); |
195 | ++ params.transition = ui::PAGE_TRANSITION_LINK; |
196 | ++ params.was_within_same_page = true; |
197 | ++ { |
198 | ++ LoadCommittedDetails details; |
199 | ++ controller_impl().RendererDidNavigate(main_test_rfh(), params, &details); |
200 | ++ EXPECT_EQ(PAGE_TYPE_ERROR, |
201 | ++ controller_impl().GetLastCommittedEntry()->GetPageType()); |
202 | ++ EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, details.type); |
203 | ++ } |
204 | ++} |
205 | ++ |
206 | + } // namespace content |
207 | |
208 | === modified file 'patches/series' |
209 | --- patches/series 2015-02-06 22:41:44 +0000 |
210 | +++ patches/series 2015-03-04 14:35:28 +0000 |
211 | @@ -29,3 +29,4 @@ |
212 | allow-os-override-for-gpu-manager.patch |
213 | viewport-min-width.patch |
214 | dont-hide-desktop-scrollbars-without-pinch-viewport.patch |
215 | +bug1423531.patch |
216 | |
217 | === modified file 'qt/tests/qmltests/api/tst_WebView_loading.qml' |
218 | --- qt/tests/qmltests/api/tst_WebView_loading.qml 2015-01-13 12:31:34 +0000 |
219 | +++ qt/tests/qmltests/api/tst_WebView_loading.qml 2015-03-04 14:35:28 +0000 |
220 | @@ -162,6 +162,29 @@ |
221 | compare(spy.count, 1, |
222 | "WebView.loading should have changed twice during the load"); |
223 | compare(expectedLoadEvents.length, 0, "Some load events are missing"); |
224 | + |
225 | + // Verify that reloading a page that triggered an error |
226 | + // generates the same sequence of load events |
227 | + spy.clear(); |
228 | + expectedLoadEvents = [ |
229 | + { type: LoadEvent.TypeStarted, url: url, loading: true }, |
230 | + { type: LoadEvent.TypeFailed, url: url, loading: true }, |
231 | + { type: LoadEvent.TypeCommitted, url: url, error: true, loading: true } |
232 | + ]; |
233 | + webView.reload(); |
234 | + |
235 | + verify(webView.loading, |
236 | + "WebView.loading should be true once we start loading"); |
237 | + compare(spy.count, 1); |
238 | + |
239 | + spy.clear(); |
240 | + spy.wait(); |
241 | + |
242 | + verify(!webView.loading, |
243 | + "WebView.loading should be false after we finish loading"); |
244 | + compare(spy.count, 1, |
245 | + "WebView.loading should have changed twice during the load"); |
246 | + compare(expectedLoadEvents.length, 0, "Some load events are missing"); |
247 | } |
248 | |
249 | function test_WebView_loading5_redirection() { |
250 | |
251 | === modified file 'qt/tests/qmltests/api/tst_WebView_save_restore_state.qml' |
252 | --- qt/tests/qmltests/api/tst_WebView_save_restore_state.qml 2014-11-20 12:37:10 +0000 |
253 | +++ qt/tests/qmltests/api/tst_WebView_save_restore_state.qml 2015-03-04 14:35:28 +0000 |
254 | @@ -149,4 +149,44 @@ |
255 | "document.querySelector('#textInput').value") == "Te$t"; }); |
256 | restored.destroy(); |
257 | } |
258 | + |
259 | + function test_WebView_save_and_restore_error_page_data() { |
260 | + return get_restore_types(); |
261 | + } |
262 | + |
263 | + // Test that restoring the state of a navigation that had triggered an error |
264 | + // triggers the error again (see https://launchpad.net/bugs/1423531). |
265 | + function test_WebView_save_and_restore_error_page(data) { |
266 | + var url = "http://invalid/"; |
267 | + webView.url = url; |
268 | + verify(webView.waitForLoadCommitted(), |
269 | + "Timed out waiting for error page"); |
270 | + |
271 | + var state = webView.currentState; |
272 | + verify(state.length > 0); |
273 | + |
274 | + var restored = webViewComponent.createObject( |
275 | + webView, {"restoreType": data.restoreType, "restoreState": state}); |
276 | + verify(restored !== null); |
277 | + |
278 | + var expectedLoadEvents = [ |
279 | + { type: LoadEvent.TypeStarted, isError: false }, |
280 | + { type: LoadEvent.TypeFailed, isError: false }, |
281 | + { type: LoadEvent.TypeCommitted, isError: true } |
282 | + ]; |
283 | + function _loadEvent(event) { |
284 | + var expected = expectedLoadEvents[0]; |
285 | + compare(event.type, expected.type); |
286 | + compare(event.url, url); |
287 | + compare(event.isError, expected.isError); |
288 | + expectedLoadEvents.shift(); |
289 | + } |
290 | + restored.loadEvent.connect(_loadEvent); |
291 | + |
292 | + tryCompare(restored, "url", url); |
293 | + restored.waitFor(function() { |
294 | + return (expectedLoadEvents.length === 0); |
295 | + }); |
296 | + restored.destroy(); |
297 | + } |
298 | } |
Thanks - I wasn't sure if you'd get a chance to do it today, so I've merged this in to 1.5