975696 - Split ArchiveReader and FileHandle implementation into separate dirs

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jan Varga [:janv]]

We now have ~60 subdirs under mozilla/dom, some of them contain just a couple of files. So I don't see a reason to share ArchiveReader and FileHandle impl in dom/file. FileSystem API implementation is going to create a new subir dom/filesystem. I propose to split dom/file into dom/archivereader and dom/filehandle.

Comment 1

[Jan Varga [:janv]]

Guys, what do you think ?

Comment 2

[Andrea Marchesini [:baku]]

+1

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Sounds ok to me and jonas.

Comment 4

[Jan Varga [:janv]]

Attachment: patch

Comment 5

[Andrea Marchesini [:baku]]

Comment on attachment 8386729 [details] [diff] [review]
patch

Review of attachment 8386729 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/archivereader/ArchiveReader.h
@@ +4,4 @@
>   * 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/. */
>  
>  #ifndef mozilla_dom_file_domarchivereader_h__

probably this should change as well... but I don't care too much.

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8386729 [details] [diff] [review]
patch

Review of attachment 8386729 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/archivereader/ArchiveReaderCommon.h
@@ +15,5 @@
> +#include "nsString.h"
> +#include "nsTArray.h"
> +
> +#define BEGIN_ARCHIVEREADER_NAMESPACE \
> +  namespace mozilla { namespace dom { namespace archivereader {

This sort of stuff has met with pretty strict resistance so I think it's better to move away from it. Let's just put archivereader and filehandle in the dom namespace? And no more FooCommon.h files since we're trying to forward-declare everything nowadays.

Comment 7

[Masatoshi Kimura [:emk]]

Yeah, the namespace should be parallel to the directory structure.

Comment 8

[Jan Varga [:janv]]

Attachment: patch v2

Ok, I tried to do my best, but I don't have time and energy for doing the same to archivereader.

So besides moving filehandle into a separate dir, the patch does:
- move stuff from mozilla::dom::filehandle to mozilla::dom
- remove FileHandleCommon.h
- forward declare as much as possible
- always include all necessary .h files
- place all includes in one section

Comment 9

[Jan Varga [:janv]]

Attachment: patch v2

rebased patch

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8410051 [details] [diff] [review]
patch v2

Review of attachment 8410051 [details] [diff] [review]:
-----------------------------------------------------------------

Cool, thanks!

Comment 11

[Jan Varga [:janv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/03fdda1af1f4

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/03fdda1af1f4