1022855 - Rename nsCSSStyleSheet to mozilla::CSSStyleSheet

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I wrote a patch for this for bug 990250, but it'll be a while before I finish that bug, and this one bitrots quickly.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Comment 2

[Cameron McCormack (:heycam)]

Comment on attachment 8437134 [details] [diff] [review]
Patch v1

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

r=me with comments addressed.  Thanks!

::: content/base/src/ShadowRoot.cpp
@@ +146,5 @@
>                               "from <style>.");
>  
>    linkingElement->SetStyleSheet(aSheet); // This sets the ownerNode on the sheet
>  
> +  nsTArray<nsRefPtr<CSSStyleSheet> >* sheets =

Use ">>" rather than "> >" now that we're allowed to, and given you make this change in at least one of the other hunks of this patch.

@@ +176,2 @@
>  {
> +  nsTArray<nsRefPtr<CSSStyleSheet> >* sheets =

And here.

@@ +746,5 @@
>  
>  uint32_t
>  ShadowRootStyleSheetList::Length()
>  {
> +  nsTArray<nsRefPtr<CSSStyleSheet> >* sheets =

And here.

::: content/base/src/nsDocument.h
@@ +802,5 @@
>    virtual void InsertStyleSheetAt(nsIStyleSheet* aSheet, int32_t aIndex) MOZ_OVERRIDE;
>    virtual void SetStyleSheetApplicableState(nsIStyleSheet* aSheet,
>                                              bool aApplicable) MOZ_OVERRIDE;
>  
> +

Don't need this blank line.

::: layout/style/CSSStyleSheet.cpp
@@ +543,5 @@
>    return mArray.IsEmpty();
>  }
>  
>  nsresult
> +nsMediaList::SetStyleSheet(CSSStyleSheet *aSheet)

Move the "*" while you're at it.

@@ +570,5 @@
>    GetText(aMediaText);
>    return NS_OK;
>  }
>  
> +// "sheet" should be an CSSStyleSheet and "doc" should be an

s/an/a/

@@ +783,5 @@
>    ChildSheetListBuilder* builder =
>      static_cast<ChildSheetListBuilder*>(aBuilder);
>  
>    // XXXbz We really need to decomtaminate all this stuff.  Is there a reason
> +  // that I can't just QI to ImportRule and get an CSSStyleSheet

s/an/a/

@@ +867,4 @@
>    mOrderedRules.EnumerateForwards(SetStyleSheetReference, nullptr);
>  }
>  
> +CSSStyleSheetInner* 

Remove trailing white space while editing this line.

@@ +1796,5 @@
>  }
>  
>  NS_IMETHODIMP    
> +CSSStyleSheet::InsertRule(const nsAString& aRule, 
> +                          uint32_t aIndex, 

Remove trailing white space from these two lines.

@@ +1820,5 @@
>  }
>  
>  nsresult
> +CSSStyleSheet::InsertRuleInternal(const nsAString& aRule, 
> +                                  uint32_t aIndex, 

And these two lines.

::: layout/style/CSSStyleSheet.h
@@ +85,5 @@
>    // currently this is the case) that any time page JS can get ts hands on a
>    // child sheet that means we've already ensured unique inners throughout its
>    // parent chain and things are good.
> +  nsRefPtr<CSSStyleSheet> mFirstChild;
> +  CORSMode      mCORSMode;

Increase or decrease the space after the type.

::: layout/style/ErrorReporter.h
@@ +17,5 @@
>  class nsCSSScanner;
>  class nsIURI;
>  
>  namespace mozilla {
> +class CSSStyleSheet;

I think I prefer to close this namespace after the CSSStyleSheet forward declaration, before reopening it for the class definitions below.

::: layout/style/GroupRule.h
@@ +22,5 @@
>  class nsMediaQueryResultCacheKey;
>  
>  namespace mozilla {
> +
> +class CSSStyleSheet;

Same here.

::: layout/style/ImportRule.h
@@ +18,5 @@
>  class nsString;
>  
>  namespace mozilla {
> +
> +class CSSStyleSheet;

And here.

::: layout/style/Loader.cpp
@@ +164,5 @@
>    // Should be 1 for non-inline sheets.
>    uint32_t                   mLineNumber;
>  
>    // The sheet we're loading data for
> +  nsRefPtr<CSSStyleSheet>  mSheet;

Fix spacing.

@@ +969,5 @@
>  }
>  
>  /* static */ PLDHashOperator
>  Loader::RemoveEntriesWithURI(URIPrincipalAndCORSModeHashKey* aKey,
> +                             nsRefPtr<CSSStyleSheet> &aSheet,

Move "&" next to type.

@@ +1049,5 @@
>    return NS_OK;
>  }
>  
>  /**
> + * CreateSheet() creates an CSSStyleSheet object for the given URI,

s/an/a/

::: layout/style/Loader.h
@@ +37,1 @@
>  namespace dom {

Would look better without the surrounding blank lines, IMO.

@@ +386,5 @@
>    friend class SheetLoadData;
>  
>    static PLDHashOperator
>    RemoveEntriesWithURI(URIPrincipalAndCORSModeHashKey* aKey,
> +                       nsRefPtr<CSSStyleSheet> &aSheet,

Move "&" next to type.

::: layout/style/Rule.h
@@ +120,5 @@
>                                                     mozilla::MallocSizeOf aMallocSizeOf,
>                                                     void* aData);
>  
>  protected:
> +  // This is either an CSSStyleSheet* or a nsHTMLStyleSheet*.  The former

s/an/an/.  And s/a nsHTMLStyleSheet/an nsHTMLStyleSheet/!

::: layout/style/nsLayoutStylesheetCache.h
@@ +17,5 @@
>  class nsIURI;
>  
>  namespace mozilla {
> +class CSSStyleSheet;
> +

Remove blank line.

::: layout/style/nsStyleSet.cpp
@@ +409,5 @@
>  
>          scope->SetIsScopedStyleRoot();
>  
>          // Create a rule processor for the scope.
> +        nsTArray< nsRefPtr<CSSStyleSheet> > sheetsForScope;

Can you remove the " "s inside the <>s while you're touching the line?

@@ +427,5 @@
>        case eDocSheet:
>        case eOverrideSheet: {
>          // levels containing CSS stylesheets (apart from eScopedDocSheet)
>          nsCOMArray<nsIStyleSheet>& sheets = mSheets[aType];
> +        nsTArray<nsRefPtr<CSSStyleSheet> > cssSheets(sheets.Count());

And remove the space in here too.

@@ +452,5 @@
>  static bool
>  IsScopedStyleSheet(nsIStyleSheet* aSheet)
>  {
> +  nsRefPtr<CSSStyleSheet> cssSheet = do_QueryObject(aSheet);
> +  NS_ASSERTION(cssSheet, "expected aSheet to be an CSSStyleSheet");

s/an/a/

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Cameron McCormack (:heycam) from comment #2)
> Comment on attachment 8437134 [details] [diff] [review]
> Patch v1
> ::: layout/style/ErrorReporter.h
> @@ +17,5 @@
> >  class nsCSSScanner;
> >  class nsIURI;
> >  
> >  namespace mozilla {
> > +class CSSStyleSheet;
> 
> I think I prefer to close this namespace after the CSSStyleSheet forward
> declaration, before reopening it for the class definitions below.

I think this actually makes more sense, given that we should be moving everything into the mozilla namespace.

Fixed your other comments.

Comment 4

[Cameron McCormack (:heycam)]

(In reply to :Ms2ger from comment #3)
> I think this actually makes more sense, given that we should be moving
> everything into the mozilla namespace.

OK.  I still think it's cleaner to separate the forward declarations completely from the main declarations below, but I don't mind it enough to argue. :-)

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

In: https://hg.mozilla.org/integration/mozilla-inbound/rev/308e51eae10c
Out: https://hg.mozilla.org/integration/mozilla-inbound/rev/3bf0b88186bf
Try: https://tbpl.mozilla.org/?tree=Try&rev=6bea55c6342f
In: https://hg.mozilla.org/integration/mozilla-inbound/rev/e9d2ef40c8d6

Comment 6

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/e9d2ef40c8d6