Last Comment Bug 539694 - accessible objects should have private copy constructor
: accessible objects should have private copy constructor
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla11
Assigned To: Jignesh Kakadiya [:jhk]
:
: alexander :surkov
Mentors:
Depends on:
Blocks: cleana11y
  Show dependency treegraph
 
Reported: 2010-01-14 07:23 PST by alexander :surkov
Modified: 2012-02-09 10:16 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Introduced copy constructor (197 bytes, patch)
2011-12-13 01:36 PST, Jignesh Kakadiya [:jhk]
no flags Details | Diff | Splinter Review
Patch_2 (905 bytes, patch)
2011-12-13 01:41 PST, Jignesh Kakadiya [:jhk]
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review
Introduced copy constructor (907 bytes, patch)
2011-12-13 02:29 PST, Jignesh Kakadiya [:jhk]
no flags Details | Diff | Splinter Review
Thanks for Pointing out. (1.43 KB, patch)
2012-02-08 10:27 PST, Jignesh Kakadiya [:jhk]
surkov.alexander: review+
Details | Diff | Splinter Review

Description alexander :surkov 2010-01-14 07:23:46 PST
Accessible objects should define copy constructor and assignment operators in private section to avoid accident calls (internal or on AT side). It was inspired by talk with Pete.
Comment 1 alexander :surkov 2011-12-07 09:11:08 PST
check all classes in folder http://mxr.mozilla.org/mozilla-central/source/accessible/src/
Comment 2 Trevor Saunders (:tbsaunde) 2011-12-07 10:34:31 PST
it appears you only need to check nsAccessNode and  not any of the things that inherit from it because having a private copy constructor / operator = means  things inheriting from you can't copy you.  Here's a test with gcc 4.6

tbsaunde@football:/tmp$ cat test.cpp
class foo
{
  public:
    foo() { }
  private:
  foo& operator =(const foo& aValue);
  foo(const foo&);
};

class dir : public foo
{
  public:
  dir() : foo() { }
  int mType;
};

  int main()
{
  dir bar;
  dir baz = bar;
  return baz.mType;
}

tbsaunde@football:/tmp$ g++ test.cpp
test.cpp: In copy constructor ‘dir::dir(const dir&)’:
test.cpp:7:3: error: ‘foo::foo(const foo&)’ is private
test.cpp:10:7: error: within this context
test.cpp: In function ‘int main()’:
test.cpp:20:13: note: synthesized method ‘dir::dir(const dir&)’ first required here
tbsaunde@football:/tmp$

sorry about the weird characters in the error message, it would appear something between bugzilla and my terminal dislikes utf8 :/
Comment 3 Jignesh Kakadiya [:jhk] 2011-12-13 01:36:26 PST
Created attachment 581202 [details] [diff] [review]
Introduced copy constructor

patch_1
Comment 4 Jignesh Kakadiya [:jhk] 2011-12-13 01:41:24 PST
Created attachment 581205 [details] [diff] [review]
Patch_2

.
Comment 5 Trevor Saunders (:tbsaunde) 2011-12-13 01:58:00 PST
Comment on attachment 581205 [details] [diff] [review]
Patch_2

r=tbsaunde not surkov
Comment 6 Jignesh Kakadiya [:jhk] 2011-12-13 02:29:00 PST
Created attachment 581211 [details] [diff] [review]
Introduced copy constructor

r = tbsaunde
Comment 7 Trevor Saunders (:tbsaunde) 2011-12-13 02:48:24 PST
Comment on attachment 581211 [details] [diff] [review]
Introduced copy constructor

review not needed since same patch.
Comment 8 Trevor Saunders (:tbsaunde) 2011-12-15 22:40:57 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/332cdb521dd6

THANKS!
Comment 9 Trevor Saunders (:tbsaunde) 2011-12-15 22:47:57 PST
actually, https://hg.mozilla.org/integration/mozilla-inbound/rev/54c446091d7c
 push race :/
Comment 10 Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-16 01:32:35 PST
Just to note, there's a new macro you should use to do this, MOZ_DELETE:

http://hg.mozilla.org/mozilla-central/annotate/3c321d2c9884/mfbt/Attributes.h#l106

It's just a matter of #include "mozilla/Attributes.h", then putting MOZ_DELETE at the ends of declarations like the ones in comment 9's URL.  This has the advantage of producing errors at compile time, not (potentially) at link time (if the code making the accidental copy is a friend of the class being copied).
Comment 11 Ed Morley [:emorley] 2011-12-16 05:51:12 PST
https://hg.mozilla.org/mozilla-central/rev/54c446091d7c

Leaving open for comment 10.
Comment 12 alexander :surkov 2012-02-07 20:48:36 PST
Jignesh, could you address comment #10 please?
Comment 13 Jignesh Kakadiya [:jhk] 2012-02-08 10:27:11 PST
Created attachment 595446 [details] [diff] [review]
Thanks for Pointing out.
Comment 14 alexander :surkov 2012-02-08 20:31:29 PST
Comment on attachment 595446 [details] [diff] [review]
Thanks for Pointing out.

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

r=me, thank you for doing this
Comment 15 alexander :surkov 2012-02-08 23:40:23 PST
inbound land https://hg.mozilla.org/integration/mozilla-inbound/rev/c6dda929759e
Comment 16 Ed Morley [:emorley] 2012-02-09 10:16:22 PST
https://hg.mozilla.org/mozilla-central/rev/c6dda929759e

Note You need to log in before you can comment on or make changes to this bug.