device storage - Move editable flag into DeviceStorageFile

RESOLVED FIXED in mozilla16

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

Trunk
mozilla16
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

12.23 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Current we are passing around a 'editable' flag in prep for 757724.  Everywhere we use this flag, we also have a DeviceStorageFile.  Lets move the flag there so that we have less to pass around.
(Assignee)

Comment 1

5 years ago
Created attachment 636259 [details] [diff] [review]
patch v.1
Assignee: nobody → doug.turner
Attachment #636259 - Flags: review?(bent.mozilla)
Comment on attachment 636259 [details] [diff] [review]
patch v.1

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

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +27,5 @@
>  public:
>  
>    nsCOMPtr<nsIFile> mFile;
>    nsString mPath;
> +  bool mEditable;

Huh, these are all public?

@@ +59,5 @@
>      NormalizeFilePath();
>    }
>  
> +  void
> +  setEditable(bool aEditable) {

Hm, everywhere you call this you do so right after constructing the object. Does it make more sense to just make this a constructor arg?

@@ +391,5 @@
>    {
>      BuildErrorString(aMessage, aFile);
>    }
>  
> +  PostErrorEvent(DOMRequest* aRequest, const nsAString& aErrorMessage)

This change looks unrelated?
(Assignee)

Comment 3

5 years ago
> Huh, these are all public?

yes. 

> Hm, everywhere you call this you do so right after constructing the object. Does it make more sense to just make this a constructor arg?

not everywhere.  the default is not editable.

> This change looks unrelated?

removed locally.
Comment on attachment 636259 [details] [diff] [review]
patch v.1

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

Ok.
Attachment #636259 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c707e4f15fd4

Comment 6

5 years ago
Push backed out for devicestorage test failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c707e4f15fd4
https://tbpl.mozilla.org/php/getParsedLog.php?id=13002287&tree=Mozilla-Inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/610161cc263b
(Assignee)

Comment 7

5 years ago
again:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/2830e6de596c
(Assignee)

Updated

5 years ago
Blocks: 761930

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/2830e6de596c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
(Assignee)

Updated

5 years ago
Blocks: 773798
You need to log in before you can comment on or make changes to this bug.