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 500123 2920174a5acf0599f64672b65e2cae110b8a6dbc
parent 500122 4ffda0cdc604ee19810a2fcfc255e1838f98c2a1
child 500124 233b7314da9d100272e895a8ce74b4490e557ac7
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1495984
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);