Closed Bug 1547854 Opened 5 years ago Closed 3 years ago

include appropriate header files in InterceptedHttpChannel

Categories

(Core :: DOM: Networking, defect, P5)

defect

Tracking

()

RESOLVED WORKSFORME

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 {                                                                                              

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

Assignee: nobody → seethesachin105
Status: NEW → ASSIGNED

I am trying to recreate the error, Can you give me a way to recreate this error

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.

Can you help me proceed?

(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 :)

Hello Mahak,
Basically the patch is in comment 0, but it has to be rebased and checked if those headers are still needed.
Thanks.

Assignee: seethesachin105 → mbansal

Thanks for the info !!
I shall try this out
Thanks :)

hey @junior
I tried rebasing the patch , did ./mach build and It completed successfully

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

Mentor: valentin.gosu

(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.

(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
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.

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 :)

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.

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:

  1. In moz.build we should move InterceptedHttpChannel.cpp from UNIFIED_SOURCES to SOURCES
  2. ./mach build and fix all the build errors by including the proper headers or using the proper namespaces when required
  3. revert the moz.build file
  4. submit the patch
See Also: → non-unified

This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Assignee: mbansal → nobody
Status: ASSIGNED → NEW

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

Flags: needinfo?(valentin.gosu)
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(valentin.gosu)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.