Bug 1495984 - Make css::URLValue::IsLocalRef call into CssUrlData::is_fragment. r=emilio
authorCameron McCormack <cam@mcc.id.au>
Wed, 17 Oct 2018 09:43:45 +0000
changeset 490054 2920174a5acf0599f64672b65e2cae110b8a6dbc
parent 490053 4ffda0cdc604ee19810a2fcfc255e1838f98c2a1
child 490055 233b7314da9d100272e895a8ce74b4490e557ac7
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersemilio
bugs1495984
milestone64.0a1
Bug 1495984 - Make css::URLValue::IsLocalRef call into CssUrlData::is_fragment. r=emilio This fixes the issue that we should no longer be looking for control characters. MozReview-Commit-ID: 8k89Aheq3NY Differential Revision: https://phabricator.services.mozilla.com/D8876
dom/base/nsContentUtils.cpp
dom/base/nsContentUtils.h
layout/style/ServoBindings.h
layout/style/nsCSSValue.cpp
layout/style/nsCSSValue.h
servo/components/style/gecko/url.rs
servo/ports/geckolib/glue.rs
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -10789,43 +10789,20 @@ nsContentUtils::GetEventTargetByLoadInfo
     }
 
     target = window->TabGroup()->EventTargetFor(aCategory);
   }
 
   return target.forget();
 }
 
-namespace {
-template<class T>
-bool IsLocalRefURL(const T& aString) {
-  // Find the first non-"C0 controls + space" character.
-  const typename T::char_type* current = aString.BeginReading();
-  for (; current != aString.EndReading(); current++) {
-    if (*current > 0x20) {
-      // if the first non-"C0 controls + space" character is '#', this is a
-      // local-ref URL.
-      return *current == '#';
-    }
-  }
-
-  return false;
-}
-}
-
 /* static */ bool
 nsContentUtils::IsLocalRefURL(const nsString& aString)
 {
-  return ::IsLocalRefURL(aString);
-}
-
-/* static */ bool
-nsContentUtils::IsLocalRefURL(const nsACString& aString)
-{
-  return ::IsLocalRefURL(aString);
+  return !aString.IsEmpty() && aString[0] == '#';
 }
 
 static const uint64_t kIdProcessBits = 32;
 static const uint64_t kIdBits = 64 - kIdProcessBits;
 
 /* static */ uint64_t
 GenerateProcessSpecificId(uint64_t aId)
 {
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -3226,23 +3226,16 @@ public:
 
   /**
    * Detect whether a string is a local-url.
    * https://drafts.csswg.org/css-values/#local-urls
    */
   static bool
   IsLocalRefURL(const nsString& aString);
 
-  /**
-   * Detect whether a string is a local-url.
-   * https://drafts.csswg.org/css-values/#local-urls
-   */
-  static bool
-  IsLocalRefURL(const nsACString& aString);
-
   static bool
   IsCustomElementsEnabled() { return sIsCustomElementsEnabled; }
 
   /**
    * Compose a tab id with process id and a serial number.
    */
   static uint64_t GenerateTabId();
 
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -1093,16 +1093,18 @@ bool Servo_HasPendingRestyleAncestor(Raw
 void Servo_CssUrlData_GetSerialization(
   RawServoCssUrlDataBorrowed url,
   uint8_t const** chars,
   uint32_t* len);
 
 RawGeckoURLExtraDataBorrowedMut Servo_CssUrlData_GetExtraData(
   RawServoCssUrlDataBorrowed url);
 
+bool Servo_CssUrlData_IsLocalRef(RawServoCssUrlDataBorrowed url);
+
 // CSS parsing utility functions.
 
 bool Servo_IsValidCSSColor(const nsAString* value);
 
 bool Servo_ComputeColor(
   RawServoStyleSetBorrowedOrNull set,
   nscolor current_color,
   const nsAString* value,
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -880,26 +880,16 @@ css::URLValue::GetURI() const
     mURI = newURI.forget();
     mURIResolved = true;
   }
 
   return mURI;
 }
 
 bool
-css::URLValue::IsLocalRef() const
-{
-  if (mIsLocalRef.isNothing()) {
-    // IsLocalRefURL is O(N), use it only when IsLocalRef is called.
-    mIsLocalRef.emplace(nsContentUtils::IsLocalRefURL(GetString()));
-  }
-  return mIsLocalRef.value();
-}
-
-bool
 css::URLValue::HasRef() const
 {
   if (IsLocalRef()) {
     return true;
   }
 
   if (nsIURI* uri = GetURI()) {
     nsAutoCString ref;
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -89,16 +89,18 @@ class CSSStyleSheet;
       dest = clm_clone;                                                        \
     }                                                                          \
   }
 
 // Forward declaration copied here since ServoBindings.h #includes nsCSSValue.h.
 extern "C" {
   RawGeckoURLExtraDataBorrowedMut Servo_CssUrlData_GetExtraData(
     RawServoCssUrlDataBorrowed url);
+
+  bool Servo_CssUrlData_IsLocalRef(RawServoCssUrlDataBorrowed url);
 }
 
 namespace mozilla {
 namespace css {
 
 struct URLValue final
 {
 public:
@@ -114,42 +116,45 @@ public:
 
   // Returns true iff all fields of the two URLValue objects are equal.
   //
   // Only safe to call on the main thread, since this will call Equals on the
   // nsIURI and nsIPrincipal objects stored on the URLValue objects.
   bool Equals(const URLValue& aOther) const;
 
   // Returns true iff we know for sure, by comparing the mBaseURI pointer,
-  // the specified url() value mString, and the mIsLocalRef, that these
+  // the specified url() value mString, and IsLocalRef(), that these
   // two URLValue objects represent the same computed url() value.
   //
   // Doesn't look at mReferrer or mOriginPrincipal.
   //
   // Safe to call from any thread.
   bool DefinitelyEqualURIs(const URLValue& aOther) const;
 
   // Smae as DefinitelyEqualURIs but additionally compares the nsIPrincipal
   // pointers of the two URLValue objects.
   bool DefinitelyEqualURIsAndPrincipal(const URLValue& aOther) const;
 
   nsIURI* GetURI() const;
 
-  bool IsLocalRef() const;
+  bool IsLocalRef() const
+  {
+    return Servo_CssUrlData_IsLocalRef(mCssUrl);
+  }
 
   bool HasRef() const;
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(URLValue)
 
-  // When matching a url with mIsLocalRef set, resolve it against aURI;
-  // Otherwise, ignore aURL and return mURL directly.
+  // When matching a local ref URL, resolve it against aURI;
+  // Otherwise, ignore aURL and return mURI directly.
   already_AddRefed<nsIURI> ResolveLocalRef(nsIURI* aURI) const;
   already_AddRefed<nsIURI> ResolveLocalRef(nsIContent* aContent) const;
 
-  // Serializes mURI as a computed URI value, taking into account mIsLocalRef
+  // Serializes mURI as a computed URI value, taking into account IsLocalRef()
   // and serializing just the fragment if true.
   void GetSourceString(nsString& aRef) const;
 
   nsDependentCSubstring GetString() const;
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   imgRequestProxy* LoadImage(nsIDocument* aDocument);
@@ -165,19 +170,16 @@ public:
 
 private:
   // mURI stores the lazily resolved URI.  This may be null if the URI is
   // invalid, even once resolved.
   mutable nsCOMPtr<nsIURI> mURI;
 
   mutable bool mURIResolved;
 
-  // mIsLocalRef is set when url starts with a U+0023 number sign(#) character.
-  mutable Maybe<bool> mIsLocalRef;
-
   RefPtr<RawServoCssUrlData> mCssUrl;
 
   const CORSMode mCORSMode;
 
   // A unique, non-reused ID value for this URLValue over the life of the
   // process.  This value is only valid after LoadImage has been called.
   //
   // We use this as a key in some tables in ImageLoader.  This is better than
--- a/servo/components/style/gecko/url.rs
+++ b/servo/components/style/gecko/url.rs
@@ -49,29 +49,36 @@ impl CssUrl {
     /// URLs in gecko, so we just return false here.
     /// use its |resolved| status.
     pub fn is_invalid(&self) -> bool {
         false
     }
 
     /// Returns true if this URL looks like a fragment.
     /// See https://drafts.csswg.org/css-values/#local-urls
+    #[inline]
     pub fn is_fragment(&self) -> bool {
-        self.as_str().chars().next().map_or(false, |c| c == '#')
+        self.0.is_fragment()
     }
 
     /// Return the unresolved url as string, or the empty string if it's
     /// invalid.
     #[inline]
     pub fn as_str(&self) -> &str {
         self.0.as_str()
     }
 }
 
 impl CssUrlData {
+    /// Returns true if this URL looks like a fragment.
+    /// See https://drafts.csswg.org/css-values/#local-urls
+    pub fn is_fragment(&self) -> bool {
+        self.as_str().chars().next().map_or(false, |c| c == '#')
+    }
+
     /// Return the unresolved url as string, or the empty string if it's
     /// invalid.
     pub fn as_str(&self) -> &str {
         &*self.serialization
     }
 }
 
 impl Parse for CssUrl {
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -5476,16 +5476,23 @@ pub unsafe extern "C" fn Servo_CssUrlDat
 #[no_mangle]
 pub extern "C" fn Servo_CssUrlData_GetExtraData(
     url: RawServoCssUrlDataBorrowed,
 ) -> *mut URLExtraData {
     CssUrlData::as_arc(&url).extra_data.0.get()
 }
 
 #[no_mangle]
+pub extern "C" fn Servo_CssUrlData_IsLocalRef(
+    url: RawServoCssUrlDataBorrowed
+) -> bool {
+    CssUrlData::as_arc(&url).is_fragment()
+}
+
+#[no_mangle]
 pub extern "C" fn Servo_ProcessInvalidations(
     set: RawServoStyleSetBorrowed,
     element: RawGeckoElementBorrowed,
     snapshots: *const ServoElementSnapshotTable,
 ) {
     debug_assert!(!snapshots.is_null());
 
     let element = GeckoElement(element);