include appropriate header files in InterceptedHttpChannel
Categories
(Core :: DOM: Networking, defect, P5)
Tracking
()
People
(Reporter: CuveeHsu, Unassigned, Mentored)
References
Details
(Keywords: good-first-bug, Whiteboard: [necko-triaged])
This is found by rebasing another project. It seems we're lack of some header files to build. I spent some time to fill them up.
--- a/netwerk/protocol/http/InterceptedHttpChannel.cpp
+++ b/netwerk/protocol/http/InterceptedHttpChannel.cpp
@@ -2,16 +2,22 @@
/* vim: set sw=2 ts=8 et tw=80 : */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "InterceptedHttpChannel.h"
#include "nsContentSecurityManager.h"
#include "nsEscape.h"
+#include "nsHttpChannel.h"
+#include "nsIRedirectResultListener.h"
+#include "mozilla/net/NeckoChannelParams.h"
+#include "nsStreamUtils.h"
+#include "nsStringStream.h"
+#include "mozilla/dom/ChannelInfo.h"
#include "mozilla/dom/PerformanceStorage.h"
namespace mozilla {
namespace net {
NS_IMPL_ISUPPORTS_INHERITED(InterceptedHttpChannel, HttpBaseChannel,
nsIInterceptedChannel, nsICacheInfoChannel,
nsIAsyncVerifyRedirectCallback, nsIRequestObserver,
diff --git a/netwerk/protocol/http/InterceptedHttpChannel.h b/netwerk/protocol/http/InterceptedHttpChannel.h
--- a/netwerk/protocol/http/InterceptedHttpChannel.h
+++ b/netwerk/protocol/http/InterceptedHttpChannel.h
@@ -5,19 +5,22 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef mozilla_net_InterceptedHttpChannel_h
#define mozilla_net_InterceptedHttpChannel_h
#include "HttpBaseChannel.h"
#include "nsINetworkInterceptController.h"
#include "nsIInputStream.h"
+#include "nsInputStreamPump.h"
+#include "nsIAsyncVerifyRedirectCallback.h"
#include "nsICacheInfoChannel.h"
#include "nsIChannelWithDivertableParentListener.h"
#include "nsIThreadRetargetableRequest.h"
+#include "nsIThreadRetargetableStreamListener.h"
namespace mozilla {
namespace net {
Comment 1•5 years ago
|
||
Hey Junior,
I am new to mozilla open source and would be interested in helping resolve this issue. Is it possible for me to be assigned to this?
I already have the firefox source code in my environment (Ubuntu 18.04) and have built it once before.
Sachin
Reporter | ||
Updated•5 years ago
|
Comment 2•5 years ago
|
||
I am trying to recreate the error, Can you give me a way to recreate this error
Reporter | ||
Comment 3•5 years ago
|
||
It's not easy to reproduce. Usually it works in current build order. (otherwise there's a build failure)
However if one changes the link order (e.g., included by another file), the lack of header matters.
What the bug's intention is to check if all the headers in description make sense.
Comment 4•5 years ago
|
||
Can you help me proceed?
Reporter | ||
Comment 5•5 years ago
|
||
(In reply to Sachin Kumar Danisetty from comment #4)
Can you help me proceed?
Please see
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide
What current stage are you?
If you haven't started, you need to clone the code, have a patch (basically in comment 0), and make it pass the build.
https://searchfox.org/ is a good tool the check things.
hey @junior
I would like to work on this !!
Can you elaborate more ?
Thanks :)
Reporter | ||
Comment 7•5 years ago
|
||
Hello Mahak,
Basically the patch is in comment 0, but it has to be rebased and checked if those headers are still needed.
Thanks.
hey @junior
I tried rebasing the patch , did ./mach build and It completed successfully
Comment 10•5 years ago
|
||
So that means I don't need any extra header file and also the existing one's are essential
So Should I Submit the patch now ?
Thanks
Mahak
Reporter | ||
Comment 11•5 years ago
|
||
(In reply to Mahak from comment #10)
So that means I don't need any extra header file and also the existing one's are essential
So Should I Submit the patch now ?
Thanks
Mahak
We pass the build of course, otherwise the test server will complain.
The thing is we rely on some building sequence so that lacking of headers doesn't cause build break.
Comment 12•5 years ago
|
||
(In reply to Junior from comment #11)
(In reply to Mahak from comment #10)
So that means I don't need any extra header file and also the existing one's are essential
So Should I Submit the patch now ?
Thanks
MahakWe pass the build of course, otherwise the test server will complain.
The thing is we rely on some building sequence so that lacking of headers doesn't cause build break.
Sorry,But I don't understand what do you mean by building sequence ?
What I understood is simply that we need to add missing header files .I tried adding some using your patch but some of them were already present and some were redundant .
Is this right or thinking in the wrong direction ?
Thanks :)
Reporter | ||
Comment 13•5 years ago
|
||
After re-thinking again, it's a WONTFIX candidate.
As you said, it's long ago and need to check again if the header is redundant or already present.
To precise, we will unify some .cpp into several big obj*//Unified_.cpp to speed up the link time.
Therefore, missing some headers could pass the build.
However, we're not always unify the same files into one.
It could has build failure if we make the unify change (usually by adding files) and we should have include some headers.
Comment 14•5 years ago
|
||
In bug 1626530 we'll be attempting to add support for unified builds again.
I think we can still try to fix this. Here are the steps:
- In moz.build we should move InterceptedHttpChannel.cpp from UNIFIED_SOURCES to SOURCES
./mach build
and fix all the build errors by including the proper headers or using the proper namespaces when required- revert the moz.build file
- submit the patch
Comment 15•4 years ago
|
||
This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.
Comment 16•3 years ago
|
||
Hello,
I am an Outreachy applicant and would like to know whether this bug has been resolved or not.If not,I would like to work on it.
Thanks,
Somdatta
Comment 17•3 years ago
|
||
This was recently fixed by https://phabricator.services.mozilla.com/D127407
Description
•