Closed Bug 1598523 Opened 5 years ago Closed 4 years ago

Use DocumentChannel for view-source loads

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla74

Tracking Flags:

Tracking

Status

firefox74

---

fixed

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(6 files)

Bug 1598523 - Share code for creating a channel for a document between nsDocShell, DocumentChannelChild and ContentChild. r?kmag 4 years ago Matt Woodrow (:mattwoodrow) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1598523 - Improve logging of channel names for DC and view-source. r?mayhemer 4 years ago Matt Woodrow (:mattwoodrow) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1598523 - Make DocumentChannelChild/ContentChild QI to the specific interfaces it wants, rather than assuming that only HttpChannelChild will have them. r?mayhemer 4 years ago Matt Woodrow (:mattwoodrow) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1598523 - Implement nsIChildChannel on nsViewSourceChannel forwarding to the inner channel. r?mayhemer 4 years ago Matt Woodrow (:mattwoodrow) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1598523 - Create nsIWrapperChannel so that we can find the inner channel within view-source. r?mayhemer 4 years ago Matt Woodrow (:mattwoodrow) 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1598523 - Enable DocumentChannel for view-source. r?kmag 4 years ago Matt Woodrow (:mattwoodrow) 47 bytes, text/x-phabricator-request		Details \| Review

Matt Woodrow (:mattwoodrow)

Assignee

Description

•

5 years ago

We currently don't use DocumentChannel for view-source, but we could/should (especially since it's usually http).

nsViewSourceChannel doesn't implement nsIChildChannel/nsIParentChannel.

I tried implementing nsIChildChannel and forwarding that to the inner channel.

This doesn't work because we create the 'real' channel as nsViewSourceChannel+HttpChannelChild. ConnectParent is forwarded to the http, which creates an HttpChannelParent in the parent process.

HttpChannelParent tries to initialize, but finds the registered channel is nsViewSourceChannel, not nsHttpChannel.

We may instead want nsViewSourceChannel to be a content-process only wrapper around DocumentChannelChild, so that all the existing DocumentChannel parts work.

Honza Bambas (:mayhemer)

Updated

•

5 years ago

Priority: -- → P3

Whiteboard: [necko-triaged]

Matt Woodrow (:mattwoodrow)

Assignee

Updated

•

4 years ago

Blocks: 1499827

Matt Woodrow (:mattwoodrow)

Assignee

Updated

•

4 years ago

Assignee: nobody → matt.woodrow

Matt Woodrow (:mattwoodrow)

Assignee

Comment 1

•

4 years ago

I thought about a couple of approaches for fixing the issues outlined in comment 0.

The first was to have DocumentChannelParent/DocumentLoadListener detect 'view-source' in the passed URI (probably via nsINestedURI), and strip it out. That way the parent-side would only create the 'inner' real channel.
When forwarding the final channel into the content process, we'd re-add the view-source prefix so that we still get nsViewSourceChannel in the child (and implement forwarding nsIChildChannel so that the inner HttpChannelChild will connect to the nsHttpChannel in the parent).

That's a bit weird though, since the channel that we'll end up exposing to the process switching selection logic won't be view-source in any way, so we won't be able to use that in our decision making.

The other alternative is to create nsViewSourceChannel, but add a way to expose the 'inner' channel, so that we can put that channel into the registar instead of the nsViewSourceChannel. That'll mean when we setup the HttpChannelParent, it'll find the inner nsHttpChannel, not the nsViewSourceChannel.

That seemed easy enough, so I tried it, and it passes test.

Matt Woodrow (:mattwoodrow)

Assignee

Comment 2

•

4 years ago

Attached file Bug 1598523 - Share code for creating a channel for a document between nsDocShell, DocumentChannelChild and ContentChild. r?kmag — Details

Note that this also implicitly adds support for the view-source+srcdoc configuration, and setting of the BaseURI, which were both in the nsDocShell version but not the others.

Depends on D57590

Matt Woodrow (:mattwoodrow)

Assignee

Comment 3

•

4 years ago

Attached file Bug 1598523 - Improve logging of channel names for DC and view-source. r?mayhemer — Details

Depends on D57887

Matt Woodrow (:mattwoodrow)

Assignee

Comment 4

•

4 years ago

Attached file Bug 1598523 - Make DocumentChannelChild/ContentChild QI to the specific interfaces it wants, rather than assuming that only HttpChannelChild will have them. r?mayhemer — Details

Depends on D57888

Matt Woodrow (:mattwoodrow)

Assignee

Comment 5

•

4 years ago

Attached file Bug 1598523 - Implement nsIChildChannel on nsViewSourceChannel forwarding to the inner channel. r?mayhemer — Details

Depends on D57890

Matt Woodrow (:mattwoodrow)

Assignee

Comment 6

•

4 years ago

Attached file Bug 1598523 - Create nsIWrapperChannel so that we can find the inner channel within view-source. r?mayhemer — Details

Depends on D57891

Matt Woodrow (:mattwoodrow)

Assignee

Comment 7

•

4 years ago

Attached file Bug 1598523 - Enable DocumentChannel for view-source. r?kmag — Details

Depends on D57893

Pulsebot

Comment 8

•

4 years ago

Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0df103c9cf3
Share code for creating a channel for a document between nsDocShell, DocumentChannelChild and ContentChild. r=kmag
https://hg.mozilla.org/integration/autoland/rev/5a22a20dd993
Improve logging of channel names for DC and view-source. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/c5abcaabaf2d
Make DocumentChannelChild/ContentChild QI to the specific interfaces it wants, rather than assuming that only HttpChannelChild will have them. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/141372c43f02
Implement nsIChildChannel on nsViewSourceChannel forwarding to the inner channel. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/6adc1062662b
Create nsIWrapperChannel so that we can find the inner channel within view-source. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/2e7ea1ae7fa2
Enable DocumentChannel for view-source. r=kmag

Bogdan Tara[:bogdan_tara | bogdant]

Comment 9

•

4 years ago

Backed out 6 changesets (Bug 1598523) for geckoview failures complaining about NavigationDelegateTest.loadUnknownProtocol

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=fbbb42a15738a7cc2eb05c17eeac20e40d8b4422&searchStr=android%2Cgeckoview-junit&tochange=04d83102a831521995c6ebc15c210d7f81c6a2d8&selectedJob=283913915

Backout link: https://hg.mozilla.org/integration/autoland/rev/04d83102a831521995c6ebc15c210d7f81c6a2d8

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=283913915&repo=autoland&lineNumber=6325

[task 2020-01-07T23:45:39.177Z] 23:45:39 [task 2020-01-07T23:46:09.221Z] 23:46:09 [task 2020-01-07T23:46:09.221Z] 23:46:09 [task 2020-01-07T23:46:09.222Z] 23:46:09 [task 2020-01-07T23:46:09.222Z] 23:46:09 [task 2020-01-07T23:46:09.222Z] 23:46:09 [task 2020-01-07T23:46:09.222Z] 23:46:09 [task 2020-01-07T23:46:09.222Z] 23:46:09 [task 2020-01-07T23:46:09.223Z] 23:46:09 [task 2020-01-07T23:46:09.223Z] 23:46:09 [task 2020-01-07T23:46:09.223Z] 23:46:09 [task 2020-01-07T23:46:09.223Z] 23:46:09 [task 2020-01-07T23:46:09.229Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 [task 2020-01-07T23:46:09.230Z] 23:46:09 INFO - TEST-START | org.mozilla.geckoview.test.NavigationDelegateTest.loadUnknownProtocol
INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=621
INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
INFO - org.mozilla.geckoview.test | Error in loadUnknownProtocol(org.mozilla.geckoview.test.NavigationDelegateTest):
INFO - org.mozilla.geckoview.test | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutRunnable.run(UiThreadUtils.java:58)
INFO - org.mozilla.geckoview.test | at android.os.Handler.handleCallback(Handler.java:751)
INFO - org.mozilla.geckoview.test | at android.os.Handler.dispatchMessage(Handler.java:95)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.util.UiThreadUtils.waitForCondition(UiThreadUtils.java:161)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1565)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1448)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1391)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1377)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.NavigationDelegateTest.testLoadEarlyErrorWithErrorPage(NavigationDelegateTest.kt:123)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.NavigationDelegateTest.testLoadEarlyError(NavigationDelegateTest.kt:137)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.NavigationDelegateTest.loadUnknownProtocol(NavigationDelegateTest.kt:205)
INFO - org.mozilla.geckoview.test | at java.lang.reflect.Method.invoke(Native Method)
INFO - org.mozilla.geckoview.test | at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
INFO - org.mozilla.geckoview.test | at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
INFO - org.mozilla.geckoview.test | at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
INFO - org.mozilla.geckoview.test | at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$2.lambda$evaluate$0$GeckoSessionTestRule$2(GeckoSessionTestRule.java:1298)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.-$$Lambda$GeckoSessionTestRule$2$sIbRNaZJgAu-QrUVWSGD8JbPSWM.run(lambda)
INFO - org.mozilla.geckoview.test | at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1950)
INFO - org.mozilla.geckoview.test | at android.os.Handler.handleCallback(Handler.java:751)
INFO - org.mozilla.geckoview.test | at android.os.Handler.dispatchMessage(Handler.java:95)
INFO - org.mozilla.geckoview.test | at android.os.Looper.loop(Looper.java:154)
INFO - org.mozilla.geckoview.test | at android.app.ActivityThread.main(ActivityThread.java:6077)
INFO - org.mozilla.geckoview.test | at java.lang.reflect.Method.invoke(Native Method)
INFO - org.mozilla.geckoview.test | at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
INFO - org.mozilla.geckoview.test | at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)
INFO - org.mozilla.geckoview.test |
INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=loadUnknownProtocol
INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.NavigationDelegateTest
INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stack=org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutRunnable.run(UiThreadUtils.java:58)
INFO - org.mozilla.geckoview.test | at android.os.Handler.handleCallback(Handler.java:751)
INFO - org.mozilla.geckoview.test | at android.os.Handler.dispatchMessage(Handler.java:95)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.util.UiThreadUtils.waitForCondition(UiThreadUtils.java:161)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1565)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1448)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1391)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1377)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.NavigationDelegateTest.testLoadEarlyErrorWithErrorPage(NavigationDelegateTest.kt:123)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.NavigationDelegateTest.testLoadEarlyError(NavigationDelegateTest.kt:137)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.NavigationDelegateTest.loadUnknownProtocol(NavigationDelegateTest.kt:205)
INFO - org.mozilla.geckoview.test | at java.lang.reflect.Method.invoke(Native Method)
INFO - org.mozilla.geckoview.test | at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
INFO - org.mozilla.geckoview.test | at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
INFO - org.mozilla.geckoview.test | at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
INFO - org.mozilla.geckoview.test | at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$2.lambda$evaluate$0$GeckoSessionTestRule$2(GeckoSessionTestRule.java:1298)
INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.-$$Lambda$GeckoSessionTestRule$2$sIbRNaZJgAu-QrUVWSGD8JbPSWM.run(lambda)
INFO - org.mozilla.geckoview.test | at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1950)
INFO - org.mozilla.geckoview.test | at android.os.Handler.handleCallback(Handler.java:751)
INFO - org.mozilla.geckoview.test | at android.os.Handler.dispatchMessage(Handler.java:95)
INFO - org.mozilla.geckoview.test | at android.os.Looper.loop(Looper.java:154)
INFO - org.mozilla.geckoview.test | at android.app.ActivityThread.main(ActivityThread.java:6077)
INFO - org.mozilla.geckoview.test | at java.lang.reflect.Method.invoke(Native Method)
INFO - org.mozilla.geckoview.test | at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
INFO - org.mozilla.geckoview.test | at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)
INFO - org.mozilla.geckoview.test |
INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=292
INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: -2
WARNING - TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.NavigationDelegateTest.loadUnknownProtocol | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
INFO - TEST-INFO took 30053ms

Flags: needinfo?(matt.woodrow)

Pulsebot

Comment 10

•

4 years ago

Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0de528fdfc4c
Share code for creating a channel for a document between nsDocShell, DocumentChannelChild and ContentChild. r=kmag
https://hg.mozilla.org/integration/autoland/rev/0a5c41c98d9b
Improve logging of channel names for DC and view-source. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/447bc9d5d15a
Make DocumentChannelChild/ContentChild QI to the specific interfaces it wants, rather than assuming that only HttpChannelChild will have them. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/0b1449f4dc1c
Implement nsIChildChannel on nsViewSourceChannel forwarding to the inner channel. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/529e61c0f598
Create nsIWrapperChannel so that we can find the inner channel within view-source. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/39c4e3bab99a
Enable DocumentChannel for view-source. r=kmag

Matt Woodrow (:mattwoodrow)

Assignee

Updated

•

4 years ago

Flags: needinfo?(matt.woodrow)

Dorel Luca [:dluca]

Comment 11

•

4 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/0de528fdfc4c
https://hg.mozilla.org/mozilla-central/rev/0a5c41c98d9b
https://hg.mozilla.org/mozilla-central/rev/447bc9d5d15a
https://hg.mozilla.org/mozilla-central/rev/0b1449f4dc1c
https://hg.mozilla.org/mozilla-central/rev/529e61c0f598
https://hg.mozilla.org/mozilla-central/rev/39c4e3bab99a

Status: NEW → RESOLVED

Closed: 4 years ago

status-firefox74: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla74

Honza Bambas (:mayhemer)

Updated

•

4 years ago

Depends on: 1608224

Chris Peterson [:cpeterson]

Updated

•

4 years ago

Depends on: 1612071

You need to log in before you can comment on or make changes to this bug.