Merge lp:~abreu-alexandre/oxide/handle-noncommitted-navigation into lp:~oxide-developers/oxide/oxide.trunk

Proposed by Alexandre Abreu on 2015-07-24
Status: Merged
Merged at revision: 1169
Proposed branch: lp:~abreu-alexandre/oxide/handle-noncommitted-navigation
Merge into: lp:~oxide-developers/oxide/oxide.trunk
Diff against target: 72 lines (+51/-1)
3 files modified
qt/tests/qmltests/crash/tst_bug1477760.html (+6/-0)
qt/tests/qmltests/crash/tst_bug1477760.qml (+41/-0)
shared/browser/oxide_web_view.cc (+4/-1)
To merge this branch: bzr merge lp:~abreu-alexandre/oxide/handle-noncommitted-navigation
Reviewer Review Type Date Requested Status
Chris Coulson 2015-07-24 Approve on 2015-07-28
Review via email: mp+265865@code.launchpad.net

Commit message

Some transient about:blank navigations are not committed so those do end up with no navigation entries whatsoever.

Description of the change

Some transient about:blank navigations are not committed so those do end up with no navigation entries whatsoever.

To post a comment you must log in.
Chris Coulson (chrisccoulson) wrote :

This isn't quite right - you should still call LoadSucceeded in this case, just with the status code set to 0. And the comment isn't accurate - the load is committed (it has to be, otherwise that's a bug), but initial renderer-initiated about:blank navigations don't get a navigation entry.

This also needs a test

review: Needs Fixing
1164. By Alexandre Abreu on 2015-07-28

Add test; fix comment

Alexandre Abreu (abreu-alexandre) wrote :

updated

Chris Coulson (chrisccoulson) wrote :

Thanks - there's a couple of comments on the test

review: Needs Fixing
1165. By Alexandre Abreu on 2015-07-28

tweak tests

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'qt/tests/qmltests/crash/tst_bug1477760.html'
2--- qt/tests/qmltests/crash/tst_bug1477760.html 1970-01-01 00:00:00 +0000
3+++ qt/tests/qmltests/crash/tst_bug1477760.html 2015-07-28 16:51:32 +0000
4@@ -0,0 +1,6 @@
5+<html>
6+ <body onclick="window.open()">
7+ <div width="100%" height="100%">
8+ </div>
9+ </body>
10+</html>
11
12=== added file 'qt/tests/qmltests/crash/tst_bug1477760.qml'
13--- qt/tests/qmltests/crash/tst_bug1477760.qml 1970-01-01 00:00:00 +0000
14+++ qt/tests/qmltests/crash/tst_bug1477760.qml 2015-07-28 16:51:32 +0000
15@@ -0,0 +1,41 @@
16+import QtQuick 2.0
17+import QtTest 1.0
18+import com.canonical.Oxide 1.0
19+import com.canonical.Oxide.Testing 1.0
20+
21+TestWebView {
22+ id: webView
23+ width: 200
24+ height: 200
25+
26+ Component {
27+ id: webViewFactory
28+ TestWebView {
29+ context: webView.context
30+ }
31+ }
32+
33+ property var created: null
34+
35+ onNewViewRequested: {
36+ created = webViewFactory.createObject(webView, { request: request });
37+ }
38+
39+ TestCase {
40+ id: test
41+ name: "bug1477760"
42+ when: windowShown
43+
44+ function test_bug1477760() {
45+ webView.url = "http://testsuite/tst_bug1477760.html";
46+ verify(webView.waitForLoadSucceeded(),
47+ "Timed out waiting for successful load");
48+
49+ mouseClick(webView, webView.width / 2, webView.height / 2);
50+
51+ TestUtils.waitFor(function() { return !!webView.created; });
52+ verify(webView.created.waitForLoadSucceeded(),
53+ "Timed out waiting for successful load");
54+ }
55+ }
56+}
57
58=== modified file 'shared/browser/oxide_web_view.cc'
59--- shared/browser/oxide_web_view.cc 2015-07-07 23:18:27 +0000
60+++ shared/browser/oxide_web_view.cc 2015-07-28 16:51:32 +0000
61@@ -1028,7 +1028,10 @@
62
63 content::NavigationEntry* entry =
64 web_contents_->GetController().GetLastCommittedEntry();
65- client_->LoadSucceeded(validated_url, entry->GetHttpStatusCode());
66+ // Some transient about:blank navigations dont have navigation entries.
67+ client_->LoadSucceeded(
68+ validated_url,
69+ entry ? entry->GetHttpStatusCode() : 0);
70 }
71
72 void WebView::DidFailLoad(content::RenderFrameHost* render_frame_host,

Subscribers

People subscribed via source and target branches